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

Define and measure duration for text notifications in milliseconds #20502

Merged
merged 2 commits into from Dec 15, 2022

Conversation

dragunoff
Copy link
Contributor

@dragunoff dragunoff commented Dec 3, 2022

Notification duration should be the same regardless of game speed. Switch to using wall-clock time defined in milliseconds instead of game ticks. Also use the opportunity to rename the field to "Duration" because "RemoveTime" is not so clear.

Closes #20501

@dragunoff dragunoff force-pushed the feature/notificatioins-wall-time branch from 91d28dd to 1c423f6 Compare December 4, 2022 10:43
@dragunoff
Copy link
Contributor Author

Update: Added update rule.

@dragunoff
Copy link
Contributor Author

Update: Moved update rule and added to the update path.

@dragunoff dragunoff force-pushed the feature/notificatioins-wall-time branch from 39a87f3 to 160c552 Compare December 4, 2022 19:30
@dragunoff
Copy link
Contributor Author

Update: fixed #20502 (comment)

PunkPun
PunkPun previously approved these changes Dec 5, 2022
Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM

@dragunoff
Copy link
Contributor Author

Update: changed field name to DisplayDurationMs.

During a game notification duration should be the same regardless of
game speed. Switch to using wall-clock time defined in milliseconds 
instead of game ticks. Also use the opportunity to rename the field 
to "Duration" because "RemoveTime" is not so clear.
@dragunoff dragunoff force-pushed the feature/notificatioins-wall-time branch from 7df81f5 to 753c6f0 Compare December 13, 2022 13:11
@dragunoff
Copy link
Contributor Author

Update: switch to measuring the time against Game.RunTime which is in milliseconds so no need to convert to ticks. This also fixes an issue where notifications won't be removed after a replay has ended.

@abcdefg30 abcdefg30 merged commit 6146030 into OpenRA:bleed Dec 15, 2022
@abcdefg30
Copy link
Member

Changelog

@dragunoff dragunoff deleted the feature/notificatioins-wall-time branch December 16, 2022 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications widget should use wall time instead of game time
5 participants