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 a suffocation gui overlay as a module - ticks left until death #1174

Merged
merged 9 commits into from
Jul 3, 2024

Conversation

Crystalwarrior
Copy link
Contributor

No description provided.

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

instead of an overlay, would it make sense to integrate this into gui/notify? we'd have to make gui/notify compatible with adventure mode, but it seems like this would be better off as a part of that tool instead of its own overlay.

gui/drowncheck.lua Outdated Show resolved Hide resolved
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

this can now be migrated to gui/notify. the message text can be a list of text tokens, so the formatting logic can remain the same.

@Crystalwarrior Crystalwarrior requested a review from myk002 June 25, 2024 13:59
gui/notify.lua Outdated
@@ -64,6 +65,11 @@ end
function NotifyOverlay:overlay_onupdate()
local choices = {}
local is_adv = dfhack.world.isAdventureMode()
if is_adv then
self.overlay_onupdate_max_freq_seconds=0
Copy link
Member

Choose a reason for hiding this comment

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

this runs on every call to plugin_onupdate, which is way too often. is 5 seconds (or even the default 10) too slow for adventure mode?

Copy link
Member

Choose a reason for hiding this comment

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

moreover, the same notification overlay will never be in both dwarfmode and adventure mode. mode-specific logic should be in the specialized subclasses below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dunno how to refactor this because something as important as your suffocation bar needs to be delivered to the player as swiftly as possible. It's especially relevant for swimming.

Perhaps it should only be updated every tick? However there's no existing logic allowing it to be updated every tick currently.

Also, what specialized subclasses do you mean? They can't affect the frequency of updating of the overlay, can they?

Copy link
Member

Choose a reason for hiding this comment

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

Also, what specialized subclasses do you mean?

I meant DwarfNotifyOverlay and AdvNotifyOverlay below, but I think this will take some more work in the framework

For now, could you revert changes to this file and instead set critical=true on the two notification types in notifications.lua?

Copy link
Member

Choose a reason for hiding this comment

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

internal/notify/notifications.lua Outdated Show resolved Hide resolved
@Crystalwarrior Crystalwarrior requested a review from myk002 July 2, 2024 23:42
@myk002 myk002 merged commit 448f20d into DFHack:master Jul 3, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants