Lua Moveset Edit - 1 Player Works Fine, More Than 1 Player Breaks Everything

[Spring E. Thing]

DOWNLOAD THE SOURCE FOR THIS POST HERE

TL;DR
My .lua moveset edit has wildly different behaviour between a server with only one player present and a server with multiple players present. I'm quite sure the reason is that I don't understand peer-to-peer network topology very well, but I need a second opinion.

I don't even know where to begin with this post, it'll be a long one I think since I've gone 'round in circles trying to deduce the problem, probably partially due to how difficult it is to get into a networking frame of mind anyway.

So I've got a project I'm working on. Doesn't particularly matter what I'm trying to achieve artistically at this early stage but clicking on the source link at the top of this post should give any casual observer an idea. Right now, all I'm expecting the mod to do is:
1. Let Mario press the Y button in a stationary action to initiate a 'windup' action

2. Let Mario press the Y button again during that 'windup' action to initiate a 'cast' action, or else default back to the vanilla 'idle' action assuming no input

3. Upon entering the 'cast' action, a do-nothing behaviour that looks like a Bob-Omb Buddy should spawn and then subsequently be deleted once Mario is out of any of the custom actions defined in this mod's .lua files (which is currently an automatic process after ~2 seconds of being in the 'cast' action).


Right now, when playing alone in a server, this is exactly what happens as shown below:

However, when at least one other player is present in the server, the logic goes all out of whack:

This disparity between the two linked pieces of footage left me with lots of questions, many I have been doing my own investigation into with little success:
A) What is the topology of the network for this game? I would assume with just how easy it is to host a game that it's peer-to-peer, but I have very little experience with peer-to-peer topology, only star topology for me. Additionally, stuff like the gPlayerSyncTable and the level of client access to what I'd consider server functions further suggests a peer-to-peer design. I guess the point of this question is how does syncing work here? If every client is authoritative and Lua is an interpreted language, then surely there's tons of room for not only desyncs but also cheating (not that cheating matters much in a game like this I suppose, but it could easily bend into otherwise malicious behaviour).

B) The only way that I am able to effectively tests .lua mods for this game is by having at least 2 clients open, one hosting a local server and one joining that local server. Both clients seem to always be listening to input even if they aren't the active window. As such, I'm wondering if half of my issues presented in the 'faulty' of the two pieces of footage above this text is down to execution order and technically receiving 2 inputs per client for every 1 input? It's a little hard to explain my thought process on this, but what I'm trying to say is that because each local client is hooking into the update loop per-Mario, then when I press some input it might be creating an echo effect in that the input happens for all Marios on one client and then again for all Marios on another despite both clients sharing their list of Marios. Would all my problems magically go away if I somehow just found a way to make only one of the two clients listen to inputs at once?

C) I'm using a call to spawn_sync_object() to spawn the behaviour that looks like a Bob-Omb Buddy. This call is made inside of a call stack hooked into an action, and so it is a call made by clients to let themselves and all other clients know to spawn something. gPlayerSyncTable cannot store the return of the spawn_sync_object() call to itself as a value to some key as the return type is the Object class with gPlayerSyncTable only being able to sync and therefore store unmanaged types like int, float, bool, etc.. As such, each Object spawned by spawn_sync_object() is tracked locally per-client through my own un-synced table indexed by the MarioState.playerIndex values of the client's player instances. My concerns here are sort of related to question A about the network topology of this game and not really understanding it. If I'm using spawn_sync_object() from one client to tell all other clients to spawn some Object, then why am I seemingly left only with the option to track/manage the existence of the spawned Object locally per-client? I totally get the limitations as to why I can't just cache the spawned Object to the gPlayerSyncTable to make the Object 'belong' to some player, what I don't get is what the alternative to that might be. To me, this'd be a great time for a star-topology network design (assuming that's not already how this works) in which a client could send out a request to the server to spawn some Object and then the server caches and tracks that Object which it orders all clients to represent graphically.

I do apologize if this post was hard to follow, I barely understand how to describe the problems I'm experiencing myself. In an ideal world, I would want all clients to send anything that should be networked as a request to the server and then for the server to have the final say on how that request is handled and then managed until further requests. This way, I can much easier visualize how I'd be able to network my code. Perhaps I can jerry-rig this to be the case with the network_send_to() function sending everything to global player index 0?

Many thanks for reading!

 

[PeachyPeach]

Hi,

So, first of all, your five variables windUpTimer_cur, castPowerNormalized, castDirEulerY, castTimer, in_act_loop_prevVal don't need to be synced between players, or rather, other players don't need to know them. Keep them client side.
Then, functions hooked to HOOK_BEFORE_MARIO_UPDATE and HOOK_MARIO_UPDATE run 16 times per frame, once for each player, even if not connected.
spawn_sync_object spawns a synchronized object for each player each time this function is called. So, if you run it inside one of the hook above, you can end up creating 16x more objects than intended.

Now, let's review the code file by file:

main.lua

detect_begin_action_loop
It is fine. Note that for boolean values, no need to do if value == true then or if value == false then, but if value then or if not value then.

detect_end_action_loop
in_act_loop_prevVal is not needed. The code can be just:

local function detect_end_action_loop(m)
    if not in_act_loop(m) then
        clean_act_loop(m)
    end
end

The following can be removed as well:

if(network_is_server()) then
    for i =0,(MAX_PLAYERS-1) do
        gPlayerSyncTable[i].windUpTimer_cur = 0
        gPlayerSyncTable[i].castPowerNormalized = 0
        gPlayerSyncTable[i].castDirEulerY = 0
        gPlayerSyncTable[i].castTimer = 0
        gPlayerSyncTable[i].in_act_loop_prevVal = false
    end
end

actions.lua

local bobberObjs = {}
If I understand correctly, this is supposed to keep track of all dummy objects spawned by players. You don't have to. It creates data redundancy and can lead to desync. Functions like obj_get_first_with_behavior are here to retrieve objects.
but for the sake of simplicity, let's keep this variable but only for the client:
local bobberObj = nil

clean_act_loop
This function deletes the bobber when the cast is finished. But it should delete only the client one, not the other ones, and only if it exists! So the code becomes:

function clean_act_loop(m)
    if m.playerIndex == 0 and bobberObj then
        obj_mark_for_deletion(bobberObj)
        bobberObj = nil
    end
end

act_gonefishin_cast_init
We're using here 3 of the 5 variables: castPowerNormalized, castDirEulerY, castTimer.
The first two seems unused for now, so declare them as local at the top of file:
local castPowerNormalized = 0
local castDirEulerY = 0
But the third one is only used in act_gonefishin_cast, and it looks like a timer used during that specific action. A Mario variable already exists for this kind of use: m.actionTimer. So let's use that one instead.
We don't want to call spawn_sync_object 16 times, but only for the client.
set_mario_animation only changes the animation, it doesn't reset it, so it's better to call it in the action function.
Finally, that function becomes:

local function act_gonefishin_cast_init(m,cPN)
    castPowerNormalized = cPN
    castDirEulerY = short_angle_to_degrees_angle(m.marioObj.header.gfx.angle.y)
    if m.playerIndex == 0 then
        bobberObj = spawn_sync_object(id_bhvGoneFishinBobber,E_MODEL_BOBOMB_BUDDY,m.pos.x,m.pos.y,m.pos.z,function()end)
    end
    set_mario_action(m,ACT_GONEFISHIN_CAST,0)
    m.actionTimer = 0
end

act_gonefishin_cast
Using m.actionTimer and calling set_mario_animation here, it becomes:

function act_gonefishin_cast(m)
    set_mario_animation(m,MARIO_ANIM_FIRST_PUNCH)
    m.actionTimer = m.actionTimer + 1
    if(m.actionTimer>=60)then
        set_mario_action(m,ACT_IDLE,0)
    end
end

enter_act_loop
Like castTimer, windUpTimer_cur is also a timer used in a specific action: Use m.actionTimer instead.
And like previously, call set_mario_animation in the action function.

function enter_act_loop(m)
    vec3f_set(m.vel,0,0,0)
    set_mario_action(m,ACT_GONEFISHIN_WINDUP,0)
    m.actionTimer = 0
end

act_gonefishin_windup
Like act_gonefishin_cast, we use m.actionTimer and call set_mario_animation here:

function act_gonefishin_windup(m)
    set_mario_animation(m,MARIO_ANIM_CREDITS_TAKE_OFF_CAP)
    local advanceCheck = m.controller.buttonPressed & GONEFISHIN_BUTTON
    if m.actionTimer > 0 and advanceCheck ~= 0 then
        act_gonefishin_cast_init(m,invlerp(0,windUpTimer_max,m.actionTimer,true))
    end
    m.actionTimer = m.actionTimer + 1
    if m.actionTimer >= windUpTimer_max then
        set_mario_action(m, ACT_IDLE, 0)
    end
end

common.lua and behaviors.lua are likely WIP, so I'll skip these ones.

To conclude some things to remember:
- Functions hooked to HOOK_BEFORE_MARIO_UPDATE and HOOK_MARIO_UPDATE run 16 times per frame, once for each player, even if not connected.
- Sync tables are expensive, use them only when really needed.
- NEVER put values that change often like timers in sync tables. They're likely to cause desync.
- spawn_sync_object spawns an object for each connected player, so make sure to call it only by the local player (m.playerIndex == 0).

 

[yes]

I have a similar problem for some reason a player playing the moveset breaks but then it doesn't break for another player that joins.

 

[Spring E. Thing]

Hi,

So, first of all, your five variables windUpTimer_cur, castPowerNormalized, castDirEulerY, castTimer, in_act_loop_prevVal don't need to be synced between players, or rather, other players don't need to know them. Keep them client side.
Then, functions hooked to HOOK_BEFORE_MARIO_UPDATE and HOOK_MARIO_UPDATE run 16 times per frame, once for each player, even if not connected.
spawn_sync_object spawns a synchronized object for each player each time this function is called. So, if you run it inside one of the hook above, you can end up creating 16x more objects than intended.

Now, let's review the code file by file:

main.lua

detect_begin_action_loop
It is fine. Note that for boolean values, no need to do if value == true then or if value == false then, but if value then or if not value then.

detect_end_action_loop
in_act_loop_prevVal is not needed. The code can be just:

local function detect_end_action_loop(m)
    if not in_act_loop(m) then
        clean_act_loop(m)
    end
end

The following can be removed as well:

if(network_is_server()) then
    for i =0,(MAX_PLAYERS-1) do
        gPlayerSyncTable[i].windUpTimer_cur = 0
        gPlayerSyncTable[i].castPowerNormalized = 0
        gPlayerSyncTable[i].castDirEulerY = 0
        gPlayerSyncTable[i].castTimer = 0
        gPlayerSyncTable[i].in_act_loop_prevVal = false
    end
end

actions.lua

local bobberObjs = {}
If I understand correctly, this is supposed to keep track of all dummy objects spawned by players. You don't have to. It creates data redundancy and can lead to desync. Functions like obj_get_first_with_behavior are here to retrieve objects.
but for the sake of simplicity, let's keep this variable but only for the client:
local bobberObj = nil

clean_act_loop
This function deletes the bobber when the cast is finished. But it should delete only the client one, not the other ones, and only if it exists! So the code becomes:

function clean_act_loop(m)
    if m.playerIndex == 0 and bobberObj then
        obj_mark_for_deletion(bobberObj)
        bobberObj = nil
    end
end

act_gonefishin_cast_init
We're using here 3 of the 5 variables: castPowerNormalized, castDirEulerY, castTimer.
The first two seems unused for now, so declare them as local at the top of file:
local castPowerNormalized = 0
local castDirEulerY = 0
But the third one is only used in act_gonefishin_cast, and it looks like a timer used during that specific action. A Mario variable already exists for this kind of use: m.actionTimer. So let's use that one instead.
We don't want to call spawn_sync_object 16 times, but only for the client.
set_mario_animation only changes the animation, it doesn't reset it, so it's better to call it in the action function.
Finally, that function becomes:

local function act_gonefishin_cast_init(m,cPN)
    castPowerNormalized = cPN
    castDirEulerY = short_angle_to_degrees_angle(m.marioObj.header.gfx.angle.y)
    if m.playerIndex == 0 then
        bobberObj = spawn_sync_object(id_bhvGoneFishinBobber,E_MODEL_BOBOMB_BUDDY,m.pos.x,m.pos.y,m.pos.z,function()end)
    end
    set_mario_action(m,ACT_GONEFISHIN_CAST,0)
    m.actionTimer = 0
end

act_gonefishin_cast
Using m.actionTimer and calling set_mario_animation here, it becomes:

function act_gonefishin_cast(m)
    set_mario_animation(m,MARIO_ANIM_FIRST_PUNCH)
    m.actionTimer = m.actionTimer + 1
    if(m.actionTimer>=60)then
        set_mario_action(m,ACT_IDLE,0)
    end
end

enter_act_loop
Like castTimer, windUpTimer_cur is also a timer used in a specific action: Use m.actionTimer instead.
And like previously, call set_mario_animation in the action function.

function enter_act_loop(m)
    vec3f_set(m.vel,0,0,0)
    set_mario_action(m,ACT_GONEFISHIN_WINDUP,0)
    m.actionTimer = 0
end

act_gonefishin_windup
Like act_gonefishin_cast, we use m.actionTimer and call set_mario_animation here:

function act_gonefishin_windup(m)
    set_mario_animation(m,MARIO_ANIM_CREDITS_TAKE_OFF_CAP)
    local advanceCheck = m.controller.buttonPressed & GONEFISHIN_BUTTON
    if m.actionTimer > 0 and advanceCheck ~= 0 then
        act_gonefishin_cast_init(m,invlerp(0,windUpTimer_max,m.actionTimer,true))
    end
    m.actionTimer = m.actionTimer + 1
    if m.actionTimer >= windUpTimer_max then
        set_mario_action(m, ACT_IDLE, 0)
    end
end

common.lua and behaviors.lua are likely WIP, so I'll skip these ones.

To conclude some things to remember:
- Functions hooked to HOOK_BEFORE_MARIO_UPDATE and HOOK_MARIO_UPDATE run 16 times per frame, once for each player, even if not connected.
- Sync tables are expensive, use them only when really needed.
- NEVER put values that change often like timers in sync tables. They're likely to cause desync.
- spawn_sync_object spawns an object for each connected player, so make sure to call it only by the local player (m.playerIndex == 0).

Incredibly thorough response! I haven't had time to read through it all yet since I pretty much just woke up, but I'll add another post once I've had a chance to with any further questions, Just wanted to let you know ASAP that I rly appreciate the help : )

 

[Spring E. Thing]

Incredibly thorough response! I haven't had time to read through it all yet since I pretty much just woke up, but I'll add another post once I've had a chance to with any further questions, Just wanted to let you know ASAP that I rly appreciate the help : )

TL;DR
Thank you so much for the help, it worked!

Alright so I've had my time to read over the post in question and implement all the advised changes. Most important of all I'd like to start off my clarifying that the suggested changes worked perfectly, so thanks a ton for that, huge burden off of my creative slate! Anyway, I was quite lucky that I took it upon myself to un-sync all the variables that should've been local myself as part of my experimentation for my own solution during the time between the post that started this thread and the response post that initiated this post, so that was a bulk of my duties towards the response post out of the way already. As for when to set the animations and use of the .actionTimer instead of bespoke local timer implementations, that seems like more of a warning than an error (though I could see the animation setting advice leaning more to an error given certain fringe cases). Nonetheless, I implemented those changes for good practice.
I think the most vital part of the response was the decision to encase the spawn_sync_object() call in a check to see if the action is being carried out by the local player specifically. It took me a bit to wrap my head around why just that part should be in a local player check, but I think I get it now. If I understand correctly: The nature of spawn_sync_object() is already telling the server to spawn one object per client wherever in the code that the call is made. By having spawn_sync_object() be a call that occurs for players on the client that AREN'T the client themselves with other clients open also calling spawn_sync_object() at those same points, it creates an echo effect that I'd struggle to put into words.

Thanks again, a tremendous help!