`NetworkWeapon.fire()` is not within Inputs.

[TheYellowArchitect]

#The below function is located Inside brawler scene, on the Weapon node
func _tick(_delta: float, _t: int):
    if input.is_firing:
        fire()

The function fire() directly changes state as it spawns a bomb projectile with velocity etc, yet the above function is exclusively a local input - it isn't shared to other players via RPCs, nor is it rollbacked. This is a design flaw.

Especially noticeable if a replay manager is to be implemented, because this forces to capture all states, instead of all inputs and the very first state.

There are 2 ways to solve this problem:

1. fire() changes the state (same as current implementation), but also becomes a dictionary key to _inputs I would suggest this key becoming button_pressed and its value to be an integer, so there can be mapped any amount of input buttons, for the same dictionary key. E.g. if there are 2 weapons to use, or more, this handles them all finely, with the condition that only 1 button can be pressed each tick (so you cannot fire both weapons at the same tick, the next weapon fire is queued for the next tick)

2. fire() changes the input directly, and the input itself is checked to spawn the bomb projectile. No idea if this will create bugs on existing code (by having the projectile spawn by state AND input, idk if there are proper checks already) ^This is already half-implemented, as the input is changed, it just isn't passed into _inputs

[TheYellowArchitect]

Turns out netfox already has input prediction/extrapolation!

UPDATE from #280 No it doesn't.

The below function is in the class rollback-synchronizer.gd:

# buffer is either _states or _inputs
func _get_history(buffer: Dictionary, tick: int) -> Dictionary:
    if buffer.has(tick):
        return buffer[tick]

    if buffer.is_empty():
        return {}

    var earliest = buffer.keys().min()
    var latest = buffer.keys().max()

    if tick < earliest:
        return buffer[earliest]

    #This is input prediction/extrapolation
        #e.g. if input movement was moving to the right
        #this will also move to the right for all future ticks (aka prediction)
    if tick > latest:
        return buffer[latest]

    var before = buffer.keys() \
        .filter(func (key): return key < tick) \
        .max()

    return buffer[before]

Currently, if input.fire() were placed in the input dictionary (is_firing is an input, but it's not stored in _inputs or sent via an RPC), it wouldn't cause practical problems because cooldown wouldn't let it fire for the next ticks. But in a machinegun/flamethrower scenario, it would bug the game.

Kinda the same with jump not causing problems, if you are already in the air, it doesnt change state, and if you just pressed it a few ticks ago, you are already in the air so change doesn't state, so the problem wasn't noticed thus far.

Basically, the input prediction/extrapolation should continue working as is, but every property should have a boolean "is extrapolateable" and if not, then have a default value (e.g. for movement, it would be Vector3.ZERO)

[elementbound]

This is a design flaw.

How?

You want the server to know you want to fire, so you call an RPC. Since it's using an RPC, there's no point to also send a bool, since the RPC does all the work. It is also not part of the rollback process indeed - I kept the projectiles out of the rollback loop to keep the code simple.

Could you please elaborate on why the above design should be changed? And also how your suggestions would solve the above? They seem to be concerned with transmitting the input, but not with the whole firing sequence ( client requests, server approves and broadcasts, client reconciles based on server response ).

---

Turns out netfox already has input prediction/extrapolation!

Kind of. If you try to grab a state or input for a tick that we don't have data for, it will try to find something that being the latest data up to that point. However, during rollback, we don't resimulate ticks that we don't have input for.

[TheYellowArchitect]

You want the server to know you want to fire, so you call an RPC. Since it's using an RPC, there's no point to also send a bool, since the RPC does all the work. It is also not part of the rollback process indeed - I kept the projectiles out of the rollback loop to keep the code simple.

The state changes without an input change. For example, a replay system cannot work by simply having the initial states, and all inputs. The input fire works locally, but isn't transmitted to others as a fire input.

And also how your suggestions would solve the above? They seem to be concerned with transmitting the input, but not with the whole firing sequence ( client requests, server approves and broadcasts, client reconciles based on server response ).

Using the 1st suggestion of the OP: Basically it becomes fully input, and the projectile is spawned for each player via input, instead of the below code. So this needs a refactor as it will break the existing logic of projectile reconcillation/rejection/acceptance, since current sends RPCs directly/instantly which change the state.

## Try to fire the weapon and return the projectile.
##
## Returns null if the weapon can't be fired.
func fire() -> Node:
    if not can_fire():
        return null

    var id: String = _generate_id()
    var projectile = _spawn()
    _save_projectile(projectile, id)
    var data = _projectile_data[id]

    if not is_multiplayer_authority():
        _request_projectile.rpc_id(get_multiplayer_authority(), id, NetworkTime.tick, data)
    else:
        _accept_projectile.rpc(id, NetworkTime.tick, data)

    _logger.debug("Calling after fire hook for %s" % [projectile.name])
    _after_fire(projectile)

    return projectile

Using the 2nd suggestion of the OP:

1. Player presses click
2. _inputs[tick] fire boolean becomes true and it changes the state by creating the projectile like currently, and also transmits this boolean input as a dummy input. This dummy input (boolean) is detected so other clients who don't own that boolean input don't try to create a state, and the game works as is.

3rd suggestion not included in the OP: Everything is kept as is, but for the replay manager, i also store the creation of every projectile with its initial values. The cleanest implementation of all, though it breaks the barrier of the seperation between states and input, but its possible, though kinda hacky.

---

Minor argument I forgot: For a game with many weapons, it would be clean to contain their firing/button press in the inputs. In the current game I'm making, most actions are integer mapped, so instead of 1 boolean for each button, its basically 1 integer which contains exclusively 1 action. Each input action is an enum. Simplifed example:

const JUMP: int = 1
const FIRE1: int = 2
const FIRE2: int = 3
const BLOCK: int = 4

So if you press crouch and fire1 and fire2, it only includes one of these actions (crouch) and queues the next action (fire1 and fire2) for next tick(s) but perhaps this design isn't very compatible with netfox.

[TheYellowArchitect]

Ah also, another argument to include it as input is that by implementing InputDelay or InputSlack, the firing of a (bomb) projectile isn't included at all.

[elementbound]

I'll return to this issue with more detail, but I have to highlight this:

For example, a replay system cannot work by simply having the initial states, and all inputs.

Currently there's no replay system. I won't optimize for systems that do not exist. Stop referring to systems that don't exist in netfox.