MantisBT - Zandronum |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0003901 | Zandronum | [All Projects] Bug | public | 2021-08-16 22:23 | 2021-09-08 02:17 |
|
| Reporter | Visual Vincent | |
| Assigned To | Kaminsky | |
| Priority | low | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | |
| Platform | Microsoft | OS | Windows | OS Version | XP/Vista/7 |
| Product Version | 3.0 | |
| Target Version | | Fixed in Version | 3.1-beta | |
|
| Summary | 0003901: PlayerNumber() returns inconsistent values when puked by the server console |
| Description | When 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 Reproduce | 1) 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 |
| Additional Information | |
| Tags | No tags attached. |
| Relationships | |
| Attached Files |  PlayerNumberTest.wad (4,794) 2021-08-16 22:23
https://zandronum.com/tracker/file_download.php?file_id=2659&type=bug |
|
| 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 | bug_revision_view_page.php?bugnote_id=21745#r13342 |
| 2021-08-17 14:37 | Visual Vincent | Note Edited: 0021745 | bug_revision_view_page.php?bugnote_id=21745#r13343 |
| 2021-08-17 19:13 | Visual Vincent | Note Edited: 0021745 | bug_revision_view_page.php?bugnote_id=21745#r13344 |
| 2021-08-17 19:13 | Visual Vincent | Note Edited: 0021745 | bug_revision_view_page.php?bugnote_id=21745#r13345 |
| 2021-08-17 19:32 | Visual Vincent | Note Edited: 0021745 | bug_revision_view_page.php?bugnote_id=21745#r13346 |
| 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 |
|
Notes |
|
|
|
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) |
|
|
|
(0021745)
|
|
Visual Vincent
|
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?
|
|
|
|
|
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. |
|
|
|
|
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. |
|
|
|
|
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. |
|
|
|
|
|
Thanks for testing it out. I'll consider it resolved then. |
|