foxssake / netfox

Addons for building multiplayer games with Godot
https://foxssake.github.io/netfox/
MIT License
379 stars 14 forks source link

Needless value set in `prepare_tick` in rollback #299

Open TheYellowArchitect opened 1 week ago

TheYellowArchitect commented 1 week ago

In RollbackSynchronizer:


func _prepare_tick(tick: int):
    # Prepare state
    #   Done individually by Rewindables ( usually Rollback Synchronizers )
    #   Restore input and state for tick
    var state: Dictionary = _get_history(_states, tick)
    var input: Dictionary = _get_history(_inputs, tick)

    PropertySnapshot.apply(state, _property_cache)
    PropertySnapshot.apply(input, _property_cache)

    for node in _nodes:
        if _can_simulate(node, tick):
            NetworkRollback.notify_simulated(node)

The above is invoked at the start of each rollback loop. But if a node cannot be simulated for that tick (e.g. NetworkTime.tick is 101 but the latest tick is 98) it still runs PropertySnapshot.apply(), despite already having the latest value. So the application will result in the same/existing result. For example, we are at NetworkTime.tick 101 and not the input authority. The input is 0,1 at tick 98 (latest_input) The ticks 99, 100 and 101 are needless, the input is already 0,1, no need to re-apply them.

An example check


func _prepare_tick(tick: int):
    # Prepare state
    #   Done individually by Rewindables ( usually Rollback Synchronizers )
    #   Restore input and state for tick
    var state: Dictionary = _get_history(_states, tick)
    var input: Dictionary = _get_history(_inputs, tick)

        if (tick <= latest_state):
            PropertySnapshot.apply(state, _property_cache)
        if (tick <= _inputs.keys.max()):
            PropertySnapshot.apply(input, _property_cache)

    for node in _nodes:
        if _can_simulate(node, tick):
            NetworkRollback.notify_simulated(node)

(btw the above latest_state variable is yet another reason for renaming which is rejected, should be latest_state_tick)

Is there a scenario where a check using the _can_simulate check earlier, causes bugs? I think so, but I cannot think of one right now. If so, this would clear user confusion (see #280), whereby it makes it clear input prediction does not happen, and also boosts performance.

albertok commented 1 week ago

This can't ever happen as the _submit_input rpcs will only ever backfill a tick if it has no data for.

So either this runs because there is new information from the past that has just come in or It's already been run and it wouldn't be in range defined in _resim_from in the first place