Ticket #41710

Various small patches - 2021/03/07

Open Date: 2021-03-08 04:15 Last Update: 2021-04-12 05:08

Reporter:
(Anonymous)
Owner:
(None)
Type:
Status:
Open
Component:
(None)
MileStone:
(None)
Priority:
5 - Medium
Severity:
5 - Medium
Resolution:
None
File:
15

Details

I was able to get a lot more finished this week. :)

There's lots of patches here, so I put them into a ZIP file to make it organized. There's some patches that fix compiler errors/warnings, including an issue that seems to happen only when compiling with newer versions of NASM. I also got player colour sets working in Zandronum now, which was actually pretty easy. I did have to add a few lines of code in a few places to make sure they worked properly, especially if the player's colour needed to be overridden for any reason. I'd maybe add a new property to BOTINFO so bots can use colour sets too.

Ticket History (3/39 Histories)

2021-03-08 04:15 Updated by: None
  • New Ticket "Various small patches - 2021/03/07" created
2021-03-08 05:35 Updated by: akmdm
Comment

I quickly made another patch, 10286.patch which adds a new property to BOTINFO to specify which colour set a bot will use, based on its name (e.g. "Green", "Gray", "Brown", "Red").

2021-03-08 06:45 Updated by: torr_samaho
Comment

Thanks a lot for the large stack of patches! I added most of them right away. Here are comments on the ones that I didn't add yet:

10273: This makes changes in ZDoom code, which should be marked with your initials somehow.

10274:

- network_CheckIfDuplicateLump: I think the DuplicateLumps.Delete( i ) will mess up the for loop logic, since by deleting position i, position i+1 will become i and will not be checked if the loop moves to i+1.

- const char *shortenedFilename = strrchr(Filename, '/') + 1;: If strrchr returns NULL, the 1 will still be added and the NULL pointer check in the next line will be false.

10275: This gives players a way to probe whether other players are ignoring them. If I remember correctly, this is not possible so far. Perhaps players may not want other player to know that they are ignoring them, so we should drop that part of the patch.

10277: I'm wondering whether one should also check "stateowner != NULL", but I'm not sure if it's necessary. A_SetTics doesn't check this either. I guess checking wouldn't hurt and we'd be on the safe side.

10280: Why do you also check "pMissile->lNetID == -1"? Do you know of a missile with the NETFL_SERVERSIDEONLY that should be spawned on the clients?

2021-03-08 06:52 Updated by: akmdm
Comment

Reply To torr_samaho

Thanks a lot for the large stack of patches! I added most of them right away. Here are comments on the ones that I didn't add yet: 10273: This makes changes in ZDoom code, which should be marked with your initials somehow. 10274: - network_CheckIfDuplicateLump: I think the DuplicateLumps.Delete( i ) will mess up the for loop logic, since by deleting position i, position i+1 will become i and will not be checked if the loop moves to i+1. - const char *shortenedFilename = strrchr(Filename, '/') + 1;: If strrchr returns NULL, the 1 will still be added and the NULL pointer check in the next line will be false. 10275: This gives players a way to probe whether other players are ignoring them. If I remember correctly, this is not possible so far. Perhaps players may not want other player to know that they are ignoring them, so we should drop that part of the patch. 10277: I'm wondering whether one should also check "stateowner != NULL", but I'm not sure if it's necessary. A_SetTics doesn't check this either. I guess checking wouldn't hurt and we'd be on the safe side. 10280: Why do you also check "pMissile->lNetID == -1"? Do you know of a missile with the NETFL_SERVERSIDEONLY that should be spawned on the clients?

Thanks for adding them in! I can quickly revise some of these if you're still able to add them in. For 10275, I guess we'll still send the private message even if the receiver has ignored the sender, but not the other way around? For 10280, I guess the Net ID check isn't necessary, I don't know of any missile with that flag that should be spawned on the clients.

2021-03-08 06:53 Updated by: akmdm
Comment
(This comment has been deleted)
2021-03-08 06:59 Updated by: torr_samaho
Comment

For 10275, I guess we'll still send the private message even if the receiver has ignored the sender, but not the other way around?

If the receiver has ignored the sender, the receiver should not get the message. The sender should just not be informed about this.

2021-03-08 07:03 Updated by: akmdm
  • File 10275_r2.patch (File ID: 6190) is attached
2021-03-08 07:04 Updated by: akmdm
  • File 10275_r2.patch (File ID: 6190) is deleted
2021-03-08 07:07 Updated by: akmdm
  • File 10275_r2.patch (File ID: 6191) is attached
2021-03-08 07:13 Updated by: akmdm
Comment

I'm starting to add revisions for the few patches that needed them. Note that I added the sanity check for A_SetTics under a separate patch: 10277_r3.patch.

2021-03-09 00:51 Updated by: akmdm
  • File 10275_r2.patch (File ID: 6191) is deleted
2021-03-09 00:51 Updated by: akmdm
  • File 10275_r2.patch (File ID: 6199) is attached
2021-03-09 00:55 Updated by: akmdm
  • File 10275_r2.patch (File ID: 6199) is deleted
2021-03-15 04:42 Updated by: torr_samaho
Comment

FYI, my desktop PC broke yesterday, so I won't be able to review any patches today. I hope I'll have everything up an running again next weekend.

2021-03-15 04:56 Updated by: akmdm
Comment

Reply To torr_samaho

FYI, my desktop PC broke yesterday, so I won't be able to review any patches today. I hope I'll have everything up an running again next weekend.

Oh, that's sucks a lot. :(

It's okay though, I totally understand. I hope everything goes okay for you.

2021-03-21 22:18 Updated by: torr_samaho
Comment

FYI, I still don't have a working desktop PC (deciding which parts to get is tricky, now is not really a good time to buy components), but I set up a usable compile environment on my laptop. So I can resume reviewing patches.

2021-03-22 03:04 Updated by: torr_samaho
Comment

10291: Can you post a small example wad that uses the proposed "gamesettings" and "removegamesetting" features? Can you elaborate the idea behind the following defines?

// AK The combined bit values of all flags that are locked in a game mode.

#define FLAGSET_VALUE 0

// AK All the bits of a flagset to be locked in a game mode.

#define FLAGSET_MASK 1

EDIT: Except for 10291, I added all patches. Thanks for the nice stack of patches :).

(Edited, 2021-03-22 03:13 Updated by: torr_samaho)
2021-03-22 03:11 Updated by: akmdm
Comment

Reply To torr_samaho

10291: Can you post a small example wad that uses the proposed "gamesettings" and "removegamesetting" features? Can you elaborate the idea behind the following defines? // AK The combined bit values of all flags that are locked in a game mode. #define FLAGSET_VALUE 0 // AK All the bits of a flagset to be locked in a game mode. #define FLAGSET_MASK 1

I uploaded the example wad gamemode_flags.wad which I used to test out these new featues. It's used for only CTF (DONTUSECOOPINFO and DONTUSESCOREBOARD are also added to this game mode, just so you know) and 1-Flag CTF.

To elaborate more on the new definitions: FLAGSET_VALUE holds the combined values of the dmflags or compatflags that were locked using the "gamesettings", and FLAGSET_MASK is just a bitfield of all the flags that need to be locked (any bit that is set to 1 is locked and cannot be manually changed using the console, while any bit that is 0 can still be changed).

By the way, I'm available on #zadev so we can discuss more there to make things easier.

EDIT: I think 10284 and 10289 were forgotten by accident. These fixed some issues with the camera pitch in OpenGL when the pitch limits were changed.

(Edited, 2021-03-22 03:33 Updated by: akmdm)
2021-03-22 04:54 Updated by: akmdm
Comment

I uploaded some more patches that are inside More Patches 2.zip.

2021-03-29 04:48 Updated by: torr_samaho
Comment

Forgot to mention this here, since we discussed this on IRC. All patches from last week except for the ones related to the movement regulator are in our repo by now. I backed out the movement regulator with "hg backout" to make this easier to review instead of applying the corresponding patch.

10301 also looks fine, so I pushed it.

2021-04-12 04:14 Updated by: torr_samaho
Comment

10304 looks good expect for one minor thing in p_acs.cpp: char command128; sprintf( command, "map %s", level.mapname ); C_DoCommand( command );

level.mapname can store string longer than what you can store in command. I think you should use an FSting for command, i.e. FString command;

command.Format( "map %s", level.mapname ); C_DoCommand( command.GetChars() );

2021-04-12 04:26 Updated by: akmdm
  • File 10304_r2.patch (File ID: 6504) is attached
2021-04-12 04:33 Updated by: akmdm
  • File 10304_r2.patch (File ID: 6504) is deleted
2021-04-12 05:08 Updated by: torr_samaho
Comment

Thanks for the new patches! I added everything except for 10309 and 10314. 10309 needs some more discussion to see on which defaults we can all agree. For 10314, weapleast doesn't seem to be working correctly online for me. I need to fire twice before the newly selected last weapon starts shooting. Perhaps because you don't use SendItemUse like weapnext, but directly try to set PendingWeapon.

Edit

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Login