search

tsiemens / pathfinder-toolkit

A Pathfinder RPG Android app, for managing characters and parties
22 stars 6 forks source link

#36 Bugfix - Moving active character up in list no longer lets previous ... #44

Open samcorcoran opened 10 years ago

samcorcoran commented 10 years ago

...character take a second turn

Moving active character from bottom to top of list allows them to continue to have current turn as cyclical order has not changed

samcorcoran commented 10 years ago

Fixes two edge cases in moving the active character. Wherever the character is moved to, the next character in the list is active next. This bugfix accounts for whether that next character's index stays the same (moved character was relocated to location above their position) or decreases (moved player moved down list).

The turn order is cyclical, so if a character is moved from last in list to first in list, their order hasn't actually changed. This could be explicitly disallowed, but at the very least the above bugfix also ensures that, if moved from bottom to top of list, the 'current turn' gets incremented by one to wrap round and end up letting the moved player still be active. I think this is the right way for this to work in regards to in-game rules (as it is a mechanical ordering of the PT list, but is meaningless in the context of the game).

tsiemens commented 10 years ago

Couple issues:

  1. Your if/else should have curly braces, regardless of the single lined nature of the contents. I prefer to follow this as a rule.
  2. The comments there are not particularly useful, since they just repeat your if statement, so please remove those. However, your description above concerning going from last position to first position not technically being and order change, and thus not warranting an actual turn incrementation would be a good comment, so please add that in the else block
tsiemens commented 10 years ago

I also forgot, you need to merge this with the release-2.2 branch, not master.