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

Fix infinite trigger count not interacting correctly with "+Interval:" #277

Merged
merged 1 commit into from
Aug 19, 2015

Conversation

MageKing17
Copy link
Member

There's an undocumented feature whereby negative trigger counts are treated as infinite. However, since it doesn't use the bump_timestamp system of positive trigger counts, it'll behave more like an infinite repeat count (e.g. with "+Interval: 5" it will only check every 5 seconds instead of becoming active again after 5 seconds have passed). This just makes negative values set bump_timestamp and excludes them from resetting the timestamp if the event didn't trigger that frame (checking MEF_USING_TRIGGER_COUNT so that the other behavior is still possible if you set a negative repeat_count and don't specify a trigger_count).

(As an addendum, FRED makes it somewhat more difficult to use negative repeat and trigger counts because it doesn't allow typing non-digit characters; you can still paste in a "-", but FRED will display e.g. 4294967295 instead of -1, even though it will save -1 in the mission file. This PR just fixes FSO's interaction with infinite trigger counts, not FRED's interface issues with them.)

There's an undocumented feature whereby negative trigger counts are treated as infinite. However, since it doesn't use the bump_timestamp system of positive trigger counts, it'll behave more like an infinite repeat count (e.g. with "+Interval: 5" it will only check every 5 seconds instead of becoming active again after 5 seconds have passed). This just makes negative values set bump_timestamp and excludes them from resetting the timestamp if the event didn't trigger that frame (checking MEF_USING_TRIGGER_COUNT so that the other behavior is still possible if you set a negative repeat_count and don't specify a trigger_count).
@MageKing17
Copy link
Member Author

For those who are curious, this is the stray commit that accidentally wound up in PR #41; removing it from there reminded me that I should probably make a PR for it.

@Karajorma
Copy link
Member

I'm not entirely sure this is an undocumented feature so much as a bug in FRED actually. Repeat count is frequently set to -1 and that is not an indication that it is meant to loop infinitely. I don't have time to look at how this is meant to work but there is a good chance that FRED occasionally also does this with trigger count, and if so, this change has just broken it. I'll look into it later today.

@MageKing17
Copy link
Member Author

There are comments that explicitly state that a repeat count of -1 means "infinite". While this is not inherently the case with trigger count (if memory serves, it defaults to -1), that's what the MEF_USING_TRIGGER_COUNT flag check is for.

As I'm unaware of any missions that actually specify a negative trigger count (everyone seems to just use 9999 or a billion or some other large positive number), I'm unaware of how this would break anything.

@Karajorma
Copy link
Member

Yeah, ran some checks and it does appear that it was repeat count that I've seen set to -1 multiple times and never trigger count. So this change should be safe.

@niffiwan niffiwan added the bug An issue from unintended consequences label Aug 17, 2015
The-E added a commit that referenced this pull request Aug 19, 2015
Fix infinite trigger count not interacting correctly with "+Interval:"
@The-E The-E merged commit 8b4f7dd into scp-fs2open:master Aug 19, 2015
@MageKing17 MageKing17 deleted the bugfix/infinite-trigger-count branch August 19, 2015 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue from unintended consequences
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants