search

CmdrDats / clj-minecraft

Clojure Bukkit plugin, Minecraft.
74 stars 35 forks source link

Change cljminecraft.blocks/extrude to pass a vector for cljminecraft.blocks/fork #39

Closed Haifen closed 11 years ago

Haifen commented 11 years ago

Currently, extrude passes a map and a vector of maps to fork, which unfortunately causes the call to fail since fork requires 1 argument exactly. This change just wraps the initial call to fork and the following & args in a vector, which fork will happily handle. This may not be perfect, but extrude works now.

What remains to be seen is if it would be better to unwrap the vector containing the maps that were passed to extrude, since I think as it stands now, fork gets handed [{:action :move :direction direction :distance c} [action_map_1 action_map_2]].

CmdrDats commented 11 years ago

Thanks for the pull request - I think thought that the issue is with fork instead of extrude, because fork should have a signature of (defn fork [& actions]) and should (apply run-actions ctx actions) instead of needing to be passed a single vector. I'm happy if you tweak it to do that instead, make a note of it in the readme.md as a breaking api change

That would make the fork function consistent with the rest of the multiple action functions in the file :)

Haifen commented 11 years ago

OK, pursuing that. However, it should be noted that cljminecraft.blocks/extrude still passes something other than {} {} {} {} to cljminecraft.blocks/fork. Since it's just passing the [& args] vector it gets, what it passes is {:action :move :direction direction :distance c} [action_map_1 action_map_2], instead of {:action :move :direction direction :distance c} action_map_1 action_map_2. Should I modify extrude to pass the latter (proper) form, or should I have fork detect nested vectors and destructure them?

Haifen commented 11 years ago

OK, this should be a little better

Haifen commented 11 years ago

Hmm, that commit still has some issues (complains that it can't resolve & as a symbol in the cljminecraft.blocks/fork definition). I'll see what's going on.

CmdrDats commented 11 years ago

Ah yes, sorry - that's an annoying gotcha with the defaction macro - it doesn't support destructuring or the & symbol because it needs names to map things to.. That's why fork took a vector of actions in the first place :/

Sorry about the pain! I should make a note of that caveat of defaction somewhere.. I haven't figured out how to support destructuring in there yet.

CmdrDats commented 11 years ago

@Haifen - did you ever figure this out? should I just close this PR without merging it?

Haifen commented 11 years ago

Hi- My apologies; I never figured this out. You may close the pull request but if I get back into trying to use clj-minecraft and solve this I might open an issue just so it's noted that there are a couple of collection type bugs that interfere with fully utilizing the REPL-based world manipulation API. It's something I think can be solved, but I lack the expertise to do so. I might try taking it up with the local Clojure gurus at the Seajure group in Seattle, WA, USA. The founder of the group is Phil Hagelberg (technomancy), the author of Leiningen and there are a number of equally experienced Clojure devs who attend. Again, sorry, and I hope this gets sorted out, but you may close the pull request. -Robin K.