Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add full support of changing merc card and its abilities #283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

piedpiper358
Copy link
Contributor

2022-11-04_15-55-04

@sebastientromp
Copy link
Contributor

Isn't there another event than GAME_STATE_UPDATE that could be used for that? That event is a crutch to give regular sync points between the logs and the app, but ideally I would like to get rid of it.
It also triggers quite frequently, so hooking events to it will lead to a lot of unnecessary computation

@piedpiper358
Copy link
Contributor Author

I tried to avoid using this event (see #279), but it is seems impossible to get complete information from other events.

@piedpiper358
Copy link
Contributor Author

We can handle merc transformation into an alternative form (CARD_CHANGED_ON_BOARD), as well as abilities that change on activation (MERCENARIES_ABILITY_ACTIVATED) but there are abilities that change when certain conditions are met (Drek'thar, Elise, Blink Fox) and their change can only be processed with GAME_STATE_UPDATE.

@piedpiper358
Copy link
Contributor Author

I double-checked it: we can use TURN_START event instead. This will work for everything except Blink Fox's Mind Thief. It is updated after start of turn.

@piedpiper358 piedpiper358 force-pushed the mercs-game-state-update branch 3 times, most recently from ae18bd7 to ad8b9c4 Compare November 7, 2022 02:08
@piedpiper358 piedpiper358 removed their assignment Nov 7, 2022
@piedpiper358
Copy link
Contributor Author

@sebastientromp, I changed handled event to TURN_START, so now it will be called only once per turn. (Blink Fox's Mind Thief ability change is also handled thanks to a small change to mercenaries-speed-parser)

@piedpiper358 piedpiper358 force-pushed the mercs-game-state-update branch from ad8b9c4 to 7f8b3b9 Compare November 9, 2022 15:14
@sebastientromp
Copy link
Contributor

Thanks. I will still need to take some time to investigate this on my side, as I would rather use dedicated events that describe what is actually happening, instead of relying on existing event side-effects.

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