Skip to content
Snippets Groups Projects

Lua: nextmapoverride & skipstats changing outside of G_ExitLevel

Merged Sal requested to merge TehRealSalt/SRB2:lua-nextmapoverride into next
1 unresolved thread

Desperately needed for SUGOI 3

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1917 1919 }
1918 1920 skipstats = lua_optboolean(L, 1);
  • Contributor

    Re: Steel's vanilla bug about tally screen always skipping, this line needs to be changed to lua_optboolean(L, 2); to get the second argument as a Boolean.

    Will yield to @MonsterIestyn 's guidance on whether to make this bugfix change, because it's technically addon breaking :thinking: but given history, he seems happy to make bugfixes even if they "break" addons.

    I may just do this myself absent MI's input, if I'm itching to start releasing...

    Edited by mazmazz
  • Author Contributor

    It's not gonna be that simple; the line lua_pop(L, 1); // pop nextmapoverride; skipstats now 1 if available is SUPPOSED to change the var num, since that's how the changing-variable-amount works. Might need to do another variable (lets say i) to keep track of which var num it is, replace the pop with i++;.

  • Contributor

    You're right; for whatever reason, lua_pop isn't working and the value passed to skipstats is actually the numeric argument for nextmapoverride.

    How I would write this is:

    if (n >= 1)
    {
    	int i = 1; // lua_pop doesn't work for some reason; use i instead
    	if (lua_isnumber(L, 1) || n >= 2)
    	{
    		nextmapoverride = (INT16)luaL_checknumber(L, i++);
    		//lua_pop(L, 1); // pop nextmapoverride; skipstats now 1 if available
    	}
    	if (lua_isboolean(L, i))
    		skipstats = luaL_checkboolean(L, i);
    }

    This tests well; I checked lua_type after the i increment and it reports as boolean (or nil, if no argument available).

    Will wait on MI's advice as to the best fix. Holding my +1 just to avoid confusion, until we get his feedback.

    (EDIT: code tweak)

    Edited by mazmazz
  • Oh, I know why it's not working, lua_pop(n) doesn't take a specific entry out of the stack, but rather "pops" n entries off the end of the stack.

    This makes most sense when you see what the function (actually a macro) is actually defined as in lua.h

    #define lua_pop(L,n) lua_settop(L, -(n)-1)

    In our case, lua_pop(1) actually becomes lua_settop(L, -2), which means the top of the stack becomes the second-to-last value ...which just means the index of nextmapoverride.

    Makes me wonder if anyone ever tried out the skiptally part of G_ExitLevel from the time Inuyasha first added the feature in, knowing this.

    Edited by Monster Iestyn
  • The actual function to remove a specific item from the stack in Lua's API is lua_remove(n), that said. If that had been used instead of lua_pop in the original code, index 1 would then be the index of skiptally as expected.

  • Contributor

    Confirmed, lua_type reports the correct type in this case. I'll merge this now then change the line myself.

  • Please register or sign in to reply
  • merged

  • mazmazz mentioned in commit dd107724

    mentioned in commit dd107724

  • mazmazz mentioned in merge request !404 (merged)

    mentioned in merge request !404 (merged)

  • Please register or sign in to reply
    Loading