Raptor007 / aq2-tng

Action Quake 2: The Next Generation. Raptor007's sandbox for testing changes. When verified stable, this code is pushed to the official aq2-tng repo:
https://github.com/aq2-tng/aq2-tng/tree/bots
4 stars 2 forks source link

AQtion changes to be aware of #133

Closed darkshade9 closed 8 months ago

darkshade9 commented 1 year ago

Hey @Raptor007 -- as I stated in another issue, my master branch is borked and I'm working to get it back to parity with yours, so I can perform proper PRs again. I've created a few changes you may be interested in, and feedback if you have any.

Adjusted serverinfo https://github.com/actionquake/aq2-tng/pull/40 With the amount of items we were putting into serverinfo, we were easily hitting the max packet lengths, so pertinent data was getting cut off. I removed all of the individual game mode fields and consolidated them under a field called gm, which is a string that is equal to the game mode name, such as gm=ctf. Similarly I added a field called gmf which works like dmflags in that the modes are additive, such as 3team darkmatch matchmode all being a possible game mode flag adjustment to the primary game mode type.

use_randoms weapon/item https://github.com/actionquake/aq2-tng/pull/39 KaniZ suggested this and did all of the legwork in the implementation, it allows the server operator to enable the use of randomized weapon/item combination rather than having the player choose. Comes in a couple of flavors, by default 0 it still has the option enabled but not enforced, and if set to 1, forces players into using a random combo, providing a new unique way to play.

Spawn items/weapons/ammo in GS_WEAPONCHOOSE game modes https://github.com/actionquake/aq2-tng/pull/41 Using the g_spawn_items server cvar, we allow the server operators to enable the spawning of items/ammo/weapons in games where you choose your weapon/item. The main driver for this was that we wanted to allow these spawns to occur in DM modes that had dm_choose 1 enabled, so that even though we did start with a unique weapon, there was ammo and weapons to be found around the map like any other deathmatch mode. What I didn't do was limit the types of ammo/weapons/items based on weapon or item bans. EDIT: The PR has been amended a few times, there was an unintended bug where it wouldn't spawn ammo for teamplay games, and another that spawned ammo in matchmode games. Those have been resolved in the aqtion branch END EDIT

Third handcannon frag message in honor of minchiAno https://github.com/actionquake/aq2-tng/pull/46 The sad passing of our of the most avid AQ2 players in recent memory left the team wanting to honor him in some meaningful way through the game he loved, and since the Handcannon was his favorite weapon, we elected to add a third frag message, included in with the other two that are chosen at random.

If you get a moment to review the code written and provide feedback on improvements or edge cases you can see, that would be awesome. Thanks

Raptor007 commented 1 year ago

When you say "borked" you mean merge conflicts? They are a pain, but better to deal with as early as possible. The longer you put it off, the more there will be to fix. I think I spent a weekend cleaning up pull #14 after @Maniac- and I had both been working all over the code. Let me know if you need help with this.

I didn't realize we were bumping into serverinfo limits, but I like your solution. Much cleaner to show one variable for the gametype than all those individual ones, and a bunch of those cvars really didn't need to be in serverinfo at all. The way you addressed it is nice because it doesn't require changing a ton of code everywhere, which would make the merge conflict much worse.

Off the top off my head, the only edge case I can think of is "was minched by" might not be detected by the kill counting script people use in Q2Pro/AQtion. Not sure though. I've never really looked into how that works.

darkshade9 commented 1 year ago

No merge conflicts, just items mistakenly added to the master branch that should not have been, like AQtion-specific stuff, Github build workflows, etc. I'm trying to see if I can just get a clean copy of your current master branch into my fork in a separate branch, and PR from there.

Raptor007 commented 1 year ago

Ah yeah I have accidentally done that too.

First I'd copy your latest source files to another directory outside version control. Then you should be able to revert the master branch to an earlier commit and go from there. You could then copy back your latest sources and use git diff to see all the changes, remove any that don't belong in master (either checkout whole files or copy-paste chunks) and then commit the ones that do. Then git checkout aqtion to switch branches, git merge master -m 'Merging master into aqtion' to update your aqtion branch from master (ignore any conflicts for now), and then again copy in your new source files. At this point git diff should only has the changes meant for aqtion, as the other changes were already committed to master. If it all looks good, now you can commit these changes to aqtion.

This process can be a bit messy so you might want to back up your entire aq2-tng repository before starting, just in case.

darkshade9 commented 1 year ago

I took the latest from your master and pushed it to a branch, then merged it into my master and picked out what didn't belong, then submitted a new PR so we can match up and start doing some proper PRs :) Let me know if stuff is borked

Raptor007 commented 1 year ago

It looks like this is a leftover from aqtion that made it into master: https://github.com/Raptor007/aq2-tng/blob/4329ecf5d618a057cbc25dee2ad08f0c44a0dd2d/source/g_save.c#L551

I'm removing this line from master as it is breaking the compile. After your next merge into aqtion, you'll probably need to re-add this line there.

Raptor007 commented 1 year ago

Merging into bots is taking a little longer than expected, because merging master tried to get rid of all the bots code. I assume at some point you accidentally merged bots into master and then removed those changes. No problem though. I can use git diff --cached file.c to confirm if there were only bot changes in that file, and then git checkout HEAD file.c to bring it back.

Going through the changes this way made me notice that sv_idleremove added the totalidletime variable, which seems to do what the idletime variable was already doing, so maybe there is some logic to refactor there.

darkshade9 commented 1 year ago

Yeah I think the bot code accidentally got in, I tried to remove it manually but your method is probably cleaner.

I found that idletime is reset when the ppl_idletime is triggered (https://github.com/Raptor007/aq2-tng/blob/master/source/p_client.c#L3343), so I went with totalidletime as a separate counter. If there's a cleaner/better way to handle it, I'd welcome any improvements.

Raptor007 commented 1 year ago

Yeah I was just looking at this more thoroughly and was about to edit my original comment. It looks like totalidletime will only increase if ppl_idletime is being used too: https://github.com/Raptor007/aq2-tng/blob/4329ecf5d618a057cbc25dee2ad08f0c44a0dd2d/source/p_client.c#L3337-L3344

I'm working on a refactor using just one variable, having it reset for sv_idleremove and just check modulo for the ppl_idletime sound.

darkshade9 commented 1 year ago

Hmm yeah good catch, I guess that's what I get for assuming a var will always be set

Raptor007 commented 1 year ago

Well, for most servers that's probably true. I think my server is unusual for not using ppl_idletime but I do intend to use sv_idleremove. And I've also made the error of accidentally merging the bots into master so don't beat yourself up over any of this. :¬)

Btw you and I both got caught by idletime appearing to be an amount of time idle, but it's actually the first level.framenum when the player had no activity. You'll see when I push the updated code, maybe tonight but more likely tomorrow.

Raptor007 commented 1 year ago

Okay I got this all done: https://github.com/Raptor007/aq2-tng/commit/92457809c90af2d3d8ec2a82c540b5af63599f41 https://github.com/Raptor007/aq2-tng/commit/f7a9bf70447c8a6146c3c674927767ee4989a157

Next time you pull, you'll need to add the server_id lines back to g_save.c and g_local.h in the aqtion branch.

darkshade9 commented 1 year ago

Hey @Raptor007 it looks like the adjustment to the sv_idleremove/ppl_idletime has caused an unexpected bug where the player will repeat the insane sounds forever, may need to turn up the volume a bit to hear

soundbug.zip

Raptor007 commented 1 year ago

I just tried and was unable to reproduce this. What are your values for sv_fps, ppl_idletime, and sv_idleremove (and any other relevant game mode settings)? Is there any new code in aqtion modifying the value of client->resp.idletime or doing a memset on client->resp? And did you have to do anything specific to trigger the bug?

darkshade9 commented 1 year ago
sv_fps 10
ppl_idletime 10
sv_idleremove 0

No code in aqtion that messes with resp.idletime that I am aware of. It happens as soon as you spawn in. I have my experimental server running it and it happens to me: useast.aq2world.com:27940

darkshade9 commented 1 year ago

Actually I think it was an issue on my end, merging didn't go as cleaning as I had hoped. Appears to work just fine on the latest build.

please ignore me

darkshade9 commented 8 months ago

Long time resolved, closing