Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChangeTeam not logged in siege #945

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

toomuchio
Copy link

Announcement suppression was preventing log entries as well, useful information since tracking team switches via ClientUserinfoChanged or spawned is unreliable, if a switch is made mid-round ect...

Announcement suppression was preventing log entries as well, useful information since tracking team switches via ClientUserinfoChanged or spawned is unreliable, if a switch is made mid-round ect...
@Razish
Copy link
Member

Razish commented Dec 8, 2017

Good call, thanks for the PR =]
As a formality, I want to fix the AppVeyor build before merging this.

@toomuchio
Copy link
Author

No worries, understood.

Would overhauling the logging system be out of the question if I was to commit something like that?

These logs are seriously dated in their format (breaking up rounds with "\n-------------\n" ect) and the lack of a plugin api makes it sometimes the easiest way to interface with the server.

I don't mind committing some time to reworking the logs with more meaningful things and removing unnecessary stuff if it's something that would be accepted.

Not sure if there's a valid reason we're sticking to the old quake 3 log format, or just nobody got to revamping it.

@Razish
Copy link
Member

Razish commented Dec 8, 2017

There has been talks about it. Issue is there are several programs used to scan the console + logs for information. Changing the format in any useful way would make these programs useless.

Perhaps this should be brought to a community discussion at JKHub. If the decision is made to adopt a new format we can start talking about how it would look like.
The next issue would be standardised adoption across mods and engines.

I think data/event oriented logging (non-informational) should be moved to an external stream ideally. I'd suggest prefixes for discriminating, but then you open the can of worms called "multi-line messages".
Catch us JKCommunity Discord if you like.

@toomuchio
Copy link
Author

Yeah that was my concern....

Instead of forcing a change like that then, we could always introduce a new log format, that can be turned on or off via a cvar. So it can be used in conjunction with the old format or replace it based on the server operators requirements.

Then perhaps overtime mods would update to support the new format and in a few years the legacy log format could be totally removed?

Razish
Razish previously approved these changes Dec 8, 2017
@ensiform
Copy link
Member

ensiform commented Dec 8, 2017

I think part of the reason for it is because of the way team switching and such works in siege.

Yes it is not good to avoid the log, but at the same time, but it could lead to log spam.

Legacy mods will unfortunately never go away, including base too. Since the majority of mods used are not maintained or are closed source that becomes an issue.

@toomuchio
Copy link
Author

That's true, so I guess the best course of action is to add a new log format that can be used in tandem.

UserInfoChanged, results in log spam this shouldn't be half as bad as that. I'd rather have a little spam and the information and none at all to be honest.
I've written a team balancing script and it flies off the handles at the moment due to not being able to track mid round switching, some mods always appear as siege to the server itself while they may be doing something else.

@ensiform
Copy link
Member

We also discovered that team changing in siege happens elsewhere anyway, which never calls that function. Unless the user or menu evokes the /team cmd

@toomuchio
Copy link
Author

Damn... Have you guys managed to figure out where it happens? Logging should be added to that as well I can amend this commit, if need be.
Really need that information to process team counts correctly for mods.

@Razish
Copy link
Member

Razish commented Sep 26, 2023

homer-appear

I'm pretty sure these are the concerning call sites?

  • OpenJK/codemp/game/g_saga.c

    Lines 644 to 651 in c1b347d

    if (ent->client->sess.sessionTeam == SIEGETEAM_TEAM1)
    {
    SetTeamQuick(ent, SIEGETEAM_TEAM2, qfalse);
    }
    else if (ent->client->sess.sessionTeam == SIEGETEAM_TEAM2)
    {
    SetTeamQuick(ent, SIEGETEAM_TEAM1, qfalse);
    }
  • OpenJK/codemp/game/g_cmds.c

    Lines 820 to 823 in c1b347d

    if (ent->client->sess.sessionTeam != ent->client->sess.siegeDesiredTeam)
    {
    SetTeamQuick(ent, ent->client->sess.siegeDesiredTeam, qfalse);
    }

@Razish Razish dismissed their stale review September 26, 2023 10:25

Concerns about Siege team changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants