pathfinder-toolkit icon indicating copy to clipboard operation
pathfinder-toolkit copied to clipboard

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

Open samcorcoran opened this issue 10 years ago • 3 comments

...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 avatar Sep 05 '14 00:09 samcorcoran

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).

samcorcoran avatar Sep 05 '14 01:09 samcorcoran

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 avatar Sep 05 '14 01:09 tsiemens

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

tsiemens avatar Sep 06 '14 20:09 tsiemens