Tweaks, a convenience method, and speculative refactor. #1

merged 5 commits into from May 23, 2012


None yet
2 participants

hypomodern commented May 23, 2012

Just taking a look at what is going on here.

I have questions. It seems like dick-all is done with the nodes specified in the template. They look like they set up a state machine governing how winners move around but this data this thrown away by the Template and Node doesn't implement any state logic. Is this to-do, or does starleagues have it and you haven't been able to extract it yet?

Do you want or need help with implementing that, is my real question, I guess.

@@ -77,10 +77,14 @@ def seed players
# This needs refactoring. Inconsistent interface
+ # Not sure if this is what you want here or not, but this now accepts hashlike input
+ # I'm also unsure about a good default position. I assume 1? - MHW

cadwallion May 23, 2012


1 will never be the correct default position because the winner is the root node, which must be n+1 where n is the highest position value on the left 'half' of the tree.

@root.payload = winner is probably sufficient for now, because I still have some math to do before I can implement automatic recalculation of node positions to map to a given binary tree representation, and I think that's a bit overkill for initial version.


cadwallion commented May 23, 2012

I would rather not assume a Hash as the node positions. I'm putting in empty hashes for the placeholders mainly because it's 1) easily serializable 2) less intense than some arbitrary object. However, I don't want to pigeon-hole the content into just a hash. SC2Battles, for example, uses a Seat object in the positions to carry extra info and to add AM::Serializer support.

Everything else looks good.


hypomodern commented May 23, 2012

There ya go.


cadwallion commented May 23, 2012

👍 🎉

cadwallion added a commit that referenced this pull request May 23, 2012

Merge pull request #1 from agoragames/tweaks_mwilson
Tweaks, a convenience method, and speculative refactor.

@cadwallion cadwallion merged commit 68be8fc into master May 23, 2012

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