Zandronum Chat on our Discord Server Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003901Zandronum[All Projects] Bugpublic2021-08-16 22:232021-09-08 02:17
ReporterVisual Vincent 
Assigned ToKaminsky 
PrioritylowSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformMicrosoftOSWindowsOS VersionXP/Vista/7
Product Version3.0 
Target VersionFixed in Version3.1-beta 
Summary0003901: PlayerNumber() returns inconsistent values when puked by the server console
DescriptionWhen puking an ACS script from the server console (or RCON), the PlayerNumber() function returns an inconsistent value depending on whether a player with number 0 is online:

  - If player 0 is online, PlayerNumber() returns 0
  - If player 0 is not online, PlayerNumber() returns -1

When executed by a non-player actor (e.g. a monster), PlayerNumber() returns -1 as it should.
Steps To Reproduce1) Download the example WAD

2) Host a server with the WAD and connect at least two clients

3) Type "puke 200" in the server console to execute the script. This gives you the following output:
    
    Expected player number: -1
    Actual player number: 0
    Console player number: -1

4) Disconnect the first player (with player number 0)

5) Puke the script from the server console again. You should now get the following output:

    Expected player number: -1
    Actual player number: -1
    Console player number: -1

6) Have the same or another player join again. They should automatically be assigned to the empty slot 0

7) Puke the script from the console yet another time. You should now see the same output as in step 3 again



-- Additional test --

To see what happens when a non-player actor executes the script, press the button in MAP01 to start the moving floor. Once the Zombie Man crosses the red bars he will trigger script 200. This should log the following output to the console:

    Expected player number: -1
    Actual player number: -1
    Console player number: -1
Attached Files? file icon PlayerNumberTest.wad [^] (4,794 bytes) 2021-08-16 22:23

- Relationships

-  Notes
User avatar (0021739)
Visual Vincent (reporter)
2021-08-16 22:30

I have tested and can confirm that this issue occurs on:

- 3.0
- 3.0.1
- 3.1 build 200501-1847
- 2.1.2 (so this isn't a new bug)
User avatar (0021745)
Visual Vincent (reporter)
2021-08-17 14:28
edited on: 2021-08-17 19:32

If it's of any help I have briefly been scouring through the latest source code, and while not having tested this with a debugger, it seems the issue could possibly be in CheckOnlinePuke in c_cmds.cpp, line 764 (shortened for readability):


static bool CheckOnlinePuke ( int script, int args[4], bool always )
{
    (...)

    if ( ( NETWORK_GetState( ) == NETSTATE_SERVER ) || ACS_IsScriptClientSide ( script ) )
    {
        // *** HERE: If 'NETWORK_GetState() == NETSTATE_SERVER' and 'consoleplayer' happens to be 0
        // for some reason, this would cause the script's activator to be Player 0.
        // Instead of trying to get a player activator, would passing NULL fix the issue?
        P_StartScript( players[consoleplayer].mo, NULL, script, level.mapname,
            args, 4, ( (script < 0 ) ? ACS_ALWAYS : 0 ) | ACS_NET );

        if ( ( NETWORK_GetState( ) == NETSTATE_SERVER ) && ( ACS_IsCalledFromConsoleCommand( ) == false ) )
        {
            SERVER_Printf( "The Server host or an RCON user is possibly cheating "
                "by calling \"puke %s %d %d %d %d\"\n",
                FBehavior::RepresentScript( script ).GetChars(),
                args[0], args[1], args[2], args[3] );
        }
    }

    (...)
}


As far as I could see a NULL activator should be supported, but I could be wrong.



EDIT:

Debugging confirms that consoleplayer indeed has a value of 0 on the server, which refers to the first player.



When player 0 disconnects, players[0].mo is set to NULL, so it seems passing NULL to P_StartScript might actually work?

Would this be a viable solution when a script is puked by the server, or are there any concerns with having a NULL activator?

User avatar (0021748)
Kaminsky (developer)
2021-08-17 23:45

Thanks for the analysis. The server doesn't make any use of the consoleplayer variable, so it should default to 0.

Since the RCON isn't really a player on the server, any scripts it pukes shouldn't treat player 0 as the activator, and the correct behaviour would be that PlayerNumber() returns -1 in all cases. Fixing this behaviour could also make it feasible to check if the RCON puked an ACS script and not just any client on the server. I'll take a look at this.
User avatar (0021749)
Visual Vincent (reporter)
2021-08-18 09:10

Glad I could assist!

Quote from "Kaminsky"
Fixing this behaviour could also make it feasible to check if the RCON puked an ACS script


Coincidentally this was how I discovered the issue. I have a script that needs to have a player activator in some instances but is callable by the RCON as well, so I was checking if PlayerNumber() < 0 to determine whether RCON was the activator or not.
User avatar (0021752)
Visual Vincent (reporter)
2021-09-07 09:41

I just tested the latest unofficial Dev build (200702-2143) and it seems to have fixed the issue. When executing the script from RCON, PlayerNumber() now returns -1 as it should.

I also tried calling functions like GiveInventory() from the same script, and it gives all players the specified item just like it usually does when the world is the activator.

In the few tests I performed, setting the activator to NULL doesn't seem to have broken anything.
User avatar (0021753)
Kaminsky (developer)
2021-09-08 02:17

Thanks for testing it out. I'll consider it resolved then.

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2021-08-16 22:23 Visual Vincent New Issue
2021-08-16 22:23 Visual Vincent File Added: PlayerNumberTest.wad
2021-08-16 22:30 Visual Vincent Note Added: 0021739
2021-08-17 14:28 Visual Vincent Note Added: 0021745
2021-08-17 14:32 Visual Vincent Note Edited: 0021745 View Revisions
2021-08-17 14:37 Visual Vincent Note Edited: 0021745 View Revisions
2021-08-17 19:13 Visual Vincent Note Edited: 0021745 View Revisions
2021-08-17 19:13 Visual Vincent Note Edited: 0021745 View Revisions
2021-08-17 19:32 Visual Vincent Note Edited: 0021745 View Revisions
2021-08-17 23:45 Kaminsky Note Added: 0021748
2021-08-17 23:45 Kaminsky Assigned To => Kaminsky
2021-08-17 23:45 Kaminsky Status new => acknowledged
2021-08-18 09:10 Visual Vincent Note Added: 0021749
2021-08-21 11:10 Kaminsky Status acknowledged => assigned
2021-09-07 09:41 Visual Vincent Note Added: 0021752
2021-09-08 02:17 Kaminsky Note Added: 0021753
2021-09-08 02:17 Kaminsky Status assigned => resolved
2021-09-08 02:17 Kaminsky Fixed in Version => 3.1-beta
2021-09-08 02:17 Kaminsky Resolution open => fixed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2021 MantisBT Team
Powered by Mantis Bugtracker