Skip to content

Undo attack#27

Open
marcopasi wants to merge 3 commits intoxosofox:masterfrom
marcopasi:undo_attack
Open

Undo attack#27
marcopasi wants to merge 3 commits intoxosofox:masterfrom
marcopasi:undo_attack

Conversation

@marcopasi
Copy link

Here is a minimal set of changes to support undoing of the last attack in the list.
The Attack model features a function that does the opposite of doDamange; the function is triggered directly by "pop"ping the AttackCollection.
Both the AttackInfo and AttackSummary views feature new "undo" clickable elements, that "pop" the AttackCollection.
The AttackList and DamageHeatMap views listen to the "remove" event of the AttackCollection. In particular AttackList re-renders at every change, so that the "undo" clickable element is present only for the last attack in the list, to avoid confusing users.

@xosofox
Copy link
Owner

xosofox commented May 29, 2013

Hey, thanks a lot for this pull request! You wrote nice and easy code, perfectly documented - looks even better than my hacks :D

Please don't be disappointed if I don't merge it directly, but maybe you can use it as a base and help me on an idea that I was already having... let me explain:

Instead of "undoing" the hit, I think it would (considering performance) still be ok to completely recalculate the attack collection form scratch, like resetting the portal to it's initial state, then re-applying the whole attack collection one by one.

This would allow some more possibilities in the future:

  • Deleting an attack from the middle
  • Modifying an attack in the middle (burster level, position?)
  • Stuff like "bulk changing" all logged attacks from L5 to L4, just keeping the positions and see the different outcomes
  • May be easier to handle IF there are multiple portals in a future version 2.0
  • Allow very crazy stuff like predefining "patterns" of attacks that a user can select, to automatically have the attack collection filled and applied ("All attacks on center", "Round robin on each reso", ...)

If you let me know if this sounds interesting for you, if you want/can do it.

If time is the limit, no problem, then I'll try to integrate your PR into the existing, but long term I'd like IPAS to provide the functionality described above and it would be cool if you could help

Again, thanks for the PR!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants