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.

Sign up for GitHub

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

Jump to bottom

Stun Expiration #613

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Stun Expiration #613

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
630e39d
Start replacement logic for Stunned countdown
dolio Sep 10, 2021
1d9e7dc
Added some explanations in stun ticker
dolio Sep 11, 2021
b85b483
Tweak initiative counting logic
dolio Sep 11, 2021
3fdd76d
Tweak/document stun tick logic
dolio Sep 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions TemplePlus/condition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4137,11 +4137,15 @@ int ConditionFunctionReplacement::StunnedInitiativeUpdate(DispatcherCallbackArgs
auto tickOn = args.GetCondArg(initIx);

if (oldInit <= newInit) {
// Initiative wrapping around from N to N+K, so don't tick
// if the target is in the interval (N+K,N]
if (newInit > tickOn && tickOn >= oldInit) {
return 0;
}
}
else {
// Initiative counting down from N+K to N, so don't tick unless
// the target is in the interval (N+K,N]
if (oldInit <= tickOn || tickOn < newInit) {
return 0;
}
Expand Down
11 changes: 9 additions & 2 deletions TemplePlus/turn_based.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,15 @@ void TurnBasedSys::InitiativeListNextActor()
dispatch.DispatcherProcessor(dispatcher, dispTypeInitiative, 0, 0);
}
}
actorInitiative = nextInitiativeIdx = 0;
InitiativeRefresh(actorInitiative, 0);
nextInitiativeIdx = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must confess I don't fully understand what was going on here, and in general these sorts of changes have to be done very carefully.
Also, I don't know if it's an issue, but have you considered/checked what happens with concurrent turns enabled?

Copy link
Contributor Author

@dolio dolio Sep 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was already changed from the original code. Can you explain why?

In the original code, it sets nextInitiativeIdx to 0, then calls InitiativeRefresh, then it sets actorInitiative to 0. The effect this has is sending a signal that counts from the current initiative to 0, so that after this when you count from 0 to the max initiative, no numbers have been skipped. In TemplePlus, instead there is a signal sent with two 0s, which was actually the reason that Stunned was broken.

However, an actual initiative can be 0 or negative, which is why I added the check after these lines, because they will cause signals that count from, say, -1 to 0, before wrapping around, which have the same bad effects as the 0->0 signal, but are harder to trigger.

The way Stunned uses these events, it shouldn't even be necessary to actually count to/past 0 before wrapping, but I don't know what else listens to these events.

I'm not sure how concurrent turns interacts with this exactly, but this is just fixing the numbers in the initiative change broadcasts, so it seems unlikely that it would be broken in ways that it wasn't already (like, counting down too far while an earlier turn is still happening, and missing some bonuses).

Copy link
Contributor Author

@dolio dolio Sep 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I should also mention, one of the reasons I fixed this is that there could otherwise be situations where someone is stunned permanently, because the stunned condition is set to tick during an initiative number that is skipped. You could do this by stunning someone as the lowest (but positive) initiative, then delaying your turn so that you have a higher initiative in the next round. Then your previous initiative number is skipped for the rest of combat.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, looks like a simple mistake. I only replaced that function for debugging apparently (the irony :P)
Looking at the condition list, Stunned is the only one that reacts to S_Initiative_Update.


// If initiative hasn't already counted down to 0 (or less), make
// that happen. Original always counted to 0 here, but that is
// problematic.
if (actorInitiative > 0) {
InitiativeRefresh(actorInitiative, 0);
actorInitiative = 0;
}
}

objHndl actorNext = objHndl::null;
Expand Down