-
Notifications
You must be signed in to change notification settings - Fork 161
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
convert event-related timestamps to new system #4633
convert event-related timestamps to new system #4633
Conversation
9a0734f
to
c072088
Compare
After extensive testing and fixing several bugs, this is now ready for review. |
This change covers just the timestamps associated with goals and events, for better review and isolation purposes. Follow-up to scp-fs2open#4569.
c072088
to
06fc8c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good.
As this is fairly deep in the logic, it's gonna need a lot of testing, but it sounds like you did that.
This interval behaviour will need to be well-documented on the respective wiki page as well.
One question though
Two situations in the event timestamp life cycle, chain delays and event status delays, require accommodation for the timestamp-plus-interval case described in `mission_process_event()`. Commit 06fc8c7 applied the fix for chain delays; this applies the same fix to event status delays. Follow-up to scp-fs2open#4633. Fixes scp-fs2open#4951.
Fix units are denominated in seconds, while timestamp units are denominated in milliseconds. This fixes the directive timing check which, after the timestamp upgrade, would time out after 5 and 7 milliseconds instead of 5 and 7 seconds. Follow-up to scp-fs2open#4633.
This change covers just the timestamps associated with goals and events, for better review and isolation purposes.
Follow-up to #4569.