Skip to content

Add GetClientOriginalLanguage#1810

Merged
Headline merged 3 commits intoalliedmodders:masterfrom
sirdigbot:reset-client-lang
Jul 29, 2022
Merged

Add GetClientOriginalLanguage#1810
Headline merged 3 commits intoalliedmodders:masterfrom
sirdigbot:reset-client-lang

Conversation

@sirdigbot
Copy link
Contributor

@sirdigbot sirdigbot commented Jul 26, 2022

It seems reasonable that while there are functions to change a client's language, there should also be function to restore it back--both with the natives and also internally.

My use case for this is I was making a language selection menu and had to implement this logic manually.
In fact, every plugin that modifies the client's language would have to do so (particularly during OnPluginEnd) to be correctly written.

It makes sense then that a correct implementation is built in natively instead.

I'm not entirely sure that I'm supposed to set it inside PlayerManager::HandleConVarQuery(), but it looks like that's just an async way of getting the client's language for certain games, so I did.

Tested with the following code

public void OnPluginStart()
{
    LoadTranslations("common.phrases");

    RegConsoleCmd("lang_set", Lang_Set);
    RegConsoleCmd("lang_reset", Lang_Reset);
}

public Action Lang_Set(int client, int args)
{
    if (args < 1)
    {
        ReplyToCommand(client, "lang_set <code>");
        return Plugin_Handled;
    }
    char arg1[32];
    GetCmdArg(1, arg1, sizeof(arg1));
    
    int lang = GetLanguageByCode(arg1);
    if (lang == -1)
    {
        ReplyToCommand(client, "bad code");
        return Plugin_Handled;
    }

    SetClientLanguage(client, lang);
    ReplyToCommand(client, "Set lang to %s", arg1);
    ReplyToCommand(client, "%t", "No matching client");
    return Plugin_Handled;
}

public Action Lang_Reset(int client, int args)
{
    ResetClientLanguage(client);
    ReplyToCommand(client, "Reset lang to default");
    ReplyToCommand(client, "%t", "No matching client");
    return Plugin_Handled;
}

asherkin
asherkin previously requested changes Jul 26, 2022
View reviewed changes
Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IGamePlayer change here will break extensions, you need to add the functions to the end of the interface and adjust the compatibility check function so that old exts can load on the new version (and new ones only on the new one).

I'm not sure it really makes sense to add ResetLanguageId / ResetClientLanguage - it seems fairly straightforward that it's just SetLanguage(GetOriginalLanguage()). I'm also not sure about the term "reset" - as the "initial" language is the server language.

Storing and exposing the original language ID definitely seems useful though.

@sirdigbot
Copy link
Contributor Author

sirdigbot commented Jul 26, 2022 •
edited
Loading

adjust the compatibility check function

Where would I find that? My guess is I would increment SMINTERFACE_PLAYERMANAGER_VERSION?

@asherkin
Copy link
Member

asherkin commented Jul 26, 2022

adjust the compatibility check function

Where would I find that? My guess is I would increment SMINTERFACE_PLAYERMANAGER_VERSION?

Yeah - look at IsVersionCompatible.

@sirdigbot
Copy link
Contributor Author

sirdigbot commented Jul 26, 2022

I think I did it?
I tested with this code and it seems to work

public void OnPluginStart()
{
    HookEvent("player_team", Event_PlayerTeam);
}

public void Event_PlayerTeam(Event event, const char[] name, bool bDontBroadcast)
{
    int client = GetClientOfUserId(event.GetInt("userid"));
    if (client == 0 || IsFakeClient(client))
        return;
    
    int original = GetOriginalLanguage(client);
    PrintToServer("LANG: %i, %i", GetClientLanguage(client), original);
    
    SetClientLanguage(client, GetLanguageByCode("pl"));
    original = GetOriginalLanguage(client);
    PrintToServer("LANG: %i, %i", GetClientLanguage(client), original);
    
    SetClientLanguage(client, original);
    PrintToServer("LANG: %i, %i", GetClientLanguage(client), original);
}

@asherkin
Copy link
Member

asherkin commented Jul 26, 2022

Yep the logic looks fine now and we're safe with the iface version bump.

I think it'd just be good to validate that it's non-breaking and my brain isn't up to checking the IsVersionCompatible logic - so if you could just grab the extensions from the latest 1.11 and 1.12 snapshot builds and ensure you can load them against your recompiled core that'd be great.

@sirdigbot
Copy link
Contributor Author

sirdigbot commented Jul 26, 2022 •
edited
Loading

Tested both sets of extensions (both 1.12.6910 and 1.11.6906) and sm exts list didn't show any failed or anything so I think that means it's good
image

@peace-maker
Copy link
Member

peace-maker commented Jul 26, 2022

I'd prefer following SMs other API naming scheme by mentioning the target of the function GetOriginalLanguage -> GetClientOriginalLanguage.

@asherkin asherkin dismissed their stale review July 26, 2022 12:57

Concerns addressed

@sirdigbot
Copy link
Contributor Author

sirdigbot commented Jul 27, 2022

I'd prefer following SMs other API naming scheme by mentioning the target of the function GetOriginalLanguage -> GetClientOriginalLanguage.

I was going to call it that originally but I thought it was too long, fixed

@dvander
Copy link
Member

dvander commented Jul 27, 2022

Don't worry, that name isn't even in the same league as Java names for length.

@peace-maker peace-maker changed the title Add ResetClientLanguage Add GetClientOriginalLanguage Jul 27, 2022
@Headline
Copy link
Member

Headline commented Jul 29, 2022

This looks good.

Thank you!

I did catch that the line below is unnecessary and can be pulled (see pPlayer->Initialize) but as far as the scope of this patch, all is well.

sourcemod/core/PlayerManager.cpp

Line 518 in b2265da

pPlayer->m_LangId = translator->GetServerLanguage();

@Headline Headline merged commit 9321229 into alliedmodders:master Jul 29, 2022
@sirdigbot sirdigbot deleted the reset-client-lang branch August 3, 2022 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peace-maker peace-maker peace-maker approved these changes

@asherkin asherkin asherkin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sirdigbot @asherkin @peace-maker @dvander @Headline