Conversation
fa8b839 to
d0ae604
Compare
DustieDog
left a comment
There was a problem hiding this comment.
Overall it feels like a good start to rewindable actions, though based of Discord discussions, I'm not sure if it'd be a good idea to include this in a feature release until more testing/development is done, even with the experimental tag.
Having mutations make an appearance in core code has me uneasy regarding #383, namely that a more robust rollback system may be increasingly hard to implement in the future. For this PR it shouldn't be an issue, but I would say this bumps up the priority on getting that bug fixed before more new features are added.
This PR seems fine to me pending resolution of the comments.
| var _state_changes: Dictionary = {} | ||
| var _queued_changes: Dictionary = {} |
There was a problem hiding this comment.
Until we're able to update to statically typed dictionaries in v2, can we comment all dictionaries with their intended data structure for readability?
| var _has_confirmed: bool = false | ||
| var _has_cancelled: bool = false | ||
|
|
||
| var _context: Dictionary = {} |
There was a problem hiding this comment.
Same as above regarding dictionary typing.
addons/netfox/rewindable-action.gd
Outdated
| _: return "?" | ||
|
|
||
| ## Toggles the action for a given [param tick] | ||
| func toggle(state: bool, tick: int = NetworkRollback.tick) -> void: |
There was a problem hiding this comment.
toggle() is confusing, as you're setting the value of state instead of actually toggling it.
state is confusing as it typically refers to states elsewhere.
There was a problem hiding this comment.
Renamed both, though "X is confusing, it usually refers to X" didn't give me much understanding on the specifics 😄
Does this make rollback projectiles spawned by weapons, possible? |
|
@TheYellowArchitect it's part of the puzzle
|
|
I am very happy to hear it :) |
|
Thanks @TheYellowArchitect, your support is much appreciated! |
Adds a node that synchronizes events that may or may not happen during specific ticks in the rollback loop. Together with Mutations, it can be used to implement weapon firing fully as part of the rollback loop:
TODOs:
NetworkHitscanWeapon3DandRewindableAction