Skip to content

Conversation

@Daniel-Hoerauf
Copy link

@Daniel-Hoerauf Daniel-Hoerauf commented Jul 20, 2017

PR for issue # .

Ready for a PR?

  • I have updated the CHANGELOG with an entry including a description, a link to my profile, and a link to the issue ticket. This change is filed under an Enhancement or Bug Fix, is shown under the relevant release tag name, and is included in my PR branch.

Implementation Notes:

Migrated wombats-lambda and created a script to manage updating the Lambda functions.

Also includes the shell of the wombats standard library to work with Lambda and running locally, but it is not yet ported to all languages or documented.

Breaking changes:


(defn add-to-state
"Update the global saved state with the given element and position"
[matrix elem]
Copy link
Contributor

@dehli dehli Jul 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably would be cleaner to destructure in the args list so you don't need the let block.

(defn add-to-state
  [matrix {:keys [x y] :as elem}]
  (assoc matrix y (assoc (nth matrix y) x elem))

If no self coordinates are provided, use distance from {:x 3 :y 3}"
([dir {x_tar :x y_tar :y} arena-half {x_self :x y_self :y}]
(case dir
"n" (and (not (= y_tar y_self)) (>= arena-half (mod (- y_self y_tar) (* arena-half 2))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not= would probably be better


(defn can-shoot-barrier?
"Returns true if there is a barrier within shooting range"
([dir arena arena-size shot-range self]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't shooting range constant? If so it might be good to have this as a constant defined up top and use it down here.

"Return true if you can move forward without a collision or poison"
[arena {x :x y :y}]
(not (in? (get-in (nth (nth arena y) x) [:contents :type])
["zakano" "wombat" "wood-barrier" "steel-barrier" "poison"])))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good for these strings to be constants up that can be used in a user's own functions / used here. Maybe something like zakano-key, etc.

"Constructs an initial global state populated by fog"
[global-size]
(add-locs (into [] (map (fn [_]
(into [] (map (fn [_] {:type "fog"}) (range global-size)))) (range global-size)))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to have this also as a constant up on top ("fog")

[arena]
(reduce
#(conj %1
(reduce
Copy link
Contributor

@dehli dehli Jul 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think map is a better method for this (and the following line). Thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that using reduce isn't the clearest method here, but how would I be able to do this using map? I'm adding the location data to each tile in the arena so I need to know the position of the element relative to the beginning of the array. I was able to accomplish this by tracking the state in reduce's accumulator, but I couldn't see how I would be able to use map

Copy link
Contributor

@dehli dehli Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you maybe use something like: map-indexed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that'd work great. I just didn't know that was an available function. Thanks!

Copy link
Contributor

@dehli dehli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions. Still need to review some more. Looks good though!

CHANGELOG.md Outdated
[/oconn]: https://github.com/oconn
[/erochest]: https://github.com/erochest
[/elibosley]: http://github.com/elibosley
[/daniel-hoerauf]: http://github.com/Daniel-Hoerauf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make this https for consistency.

@@ -0,0 +1,34 @@
// Import the standard library and add elper functions to the same namespace
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[h]elper


(defn get-arena-size
"Fetches the size of one side of the arena from the state"
[state]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm not entirely sure that I like this more but figured I'd show some additional options. Feel free to use whichever you like the best.

(defn get-arena-size
  [{[width height] :global-dimensions}]
  {:width width
   :height height})

size (get-arena-size state)]
(if (nil? saved)
(build-initial-global-state size)
saved )))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space here

2 :about-face
3 :right)))

(defn can-shoot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make this end with a ? since it returns a boolean.

y: local_state['global-coords'][1]
};

for (i = 0; i < local_tiles.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have to use a for loop, you should declare the variable.

for (let i = 0; i < ...

};

this.get_global_state = function(state, path) {
var temp = state;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you trying to do with temp here?

break;
case 'w':
return (target.x != wombat.x) && (x_half >= mod(wombat.x - target.x, (x_half * 2)));
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

break isn't needed since you already have returned.

break;
case 'e':
return wom.y == tile.y && (shot_range >= mod(tile.x - wom.x, arena_size.width));
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

break not needed.

return wom.x == tile.x && (shot_range >= mod(tile.y - wom.y, arena_size.height));
break;
case 'w':
return wom.x == tile.x && (shot_range >= mod(wom.y - tile.y, arena_size.width));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

===

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants