Skip to content

Add some timed actions documentation - #3

Draft
demiurgeQuantified wants to merge 3 commits into
PZ-Umbrella:mainfrom
demiurgeQuantified:timedactions
Draft

Add some timed actions documentation#3
demiurgeQuantified wants to merge 3 commits into
PZ-Umbrella:mainfrom
demiurgeQuantified:timedactions

Conversation

@demiurgeQuantified

Copy link
Copy Markdown
Member

Adds notes to some timed action methods, focused around networking. This adds the helper class umbrella.NetworkedTimedAction: it would be cool to auto-assign this as a base class to networked vanilla actions by searching for complete() methods, but as is it's already helpful for user code to subclass.

@omarkmu

omarkmu commented Dec 12, 2025

Copy link
Copy Markdown
Member

This looks good to me—out of curiosity, why is animEvent kept in ISBaseTimedAction as an extra instead of being moved to umbrella.NetworkedTimedAction with the rest?

@demiurgeQuantified

Copy link
Copy Markdown
Member Author

It's not related to the new network model so it didn't fit there: it's also an optional function that the base class doesn't implement but it felt really excessive to define one function as a helper class. Thinking on it now, it's probably not great as it'll make the typings for the vanilla classes inaccurate, this is a really annoying habit in the timed action code 😓 I'm not sure now how to go about documenting these optional functions without impacting accuracy, maybe tiny helper classes are the only way. We can just remove it for now if necessary.

@omarkmu

omarkmu commented Dec 12, 2025

Copy link
Copy Markdown
Member

I handled some of the optional methods in ISUIElement using fields. I'm fine with removing it entirely, but the field approach is also an option (albeit a subpar one that I'm not a huge fan of either).

@demiurgeQuantified
demiurgeQuantified marked this pull request as draft August 9, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants