Player Colors set by LUA not working correctly

[matony1989]

I am trying to assign colors to characters (portraits) in custom campaign. It would be more memorable to know when you are fighting “red” opponent, that it is always the same “character”. What I noticed is that colors set in LUAs are acting strange.

Tested on versions 20260124 and 20250626

luas and maps can be downloaded here:
Eric Ver1.1.zip

Color index :
0 Blue
1 Yellow
2 Red
3 Pink
4 Grey
5 Green
6 Orange
7 Turquoise
8 White
9 Brown
10 Purple

Mission 2 example:
Player 1 = white (8)
Player 2 = red (2)
Player 3 = turquoise (7)

8(white) 2(red) 7(turquoise) results in 8(white) 3(pink) 7(turquoise) in game menu

I tried :
8 7 2 => 8 7 2 correct – Just changed order of colors (cannot be used in every mission)
8 0 7 => 8 0 7 correct
8 1 7 => 8 1 7 correct
8 3 7 => 8 3 7 correct ( same result as 8 2 7)
8 4 7 => 8 4 7 correct (Initially I thought grey is causing issues, you will see why later)
5, 6, 9 , 10 correct

All colors are working, except 2-red for map 2 on player position 2.
Tried also 8 7 9 – result 8 7 9 correct

Map 3:
8 0 2 => 8 0 2 Correct
Map 4:
8 5 2 7 => 8 5 2 7 Correct
Map 5 same setup as map 2, same result
Map 6
8 1 2 6 => 8 1 2 6 Correct
Map 7
8 10 2 => 8 10 2 Correct
Tried 8 2 10 => 8 3 10

Map 8
8 0 2 7 5 => correct

Map 9
8 1 6 2 => correct
Map 10
8 10 5 4 2 => 8 10 5 6 2 – This is where I first noticed it. Instead of Grey 4 I got first available color from 4 up = 6
Map 11
8 10 1 2 7 - correct
Map 12
8 3 9 2 7 => 8 5 9 2 7
Map 13
8 5 2 4 0 => 8 5 2 6 0
Map 14
8 1 2 3 6 7 4 => 8 1 2 3 7 9 4
Map 15
8 10 2 7 9 0 6 5 => 8 10 2 9 0 1 6 5
0 1 2 3 4 5 6 7 – correct

Anyone know what might be causing this? There are some weird patterns to this I believe. At first I noticed it only on maps where I used Grey color, but it also happens when there is Red and color skips. Also it happens with higher amount of players on certain player slots.

I have attached also visualization and scripts with maps if anyone wants to test it

[Flamefire]

I'm not sure I understand correctly. What exactly are the numbers you listed for the maps? I.e. what does this mean?

Map 14
8 1 2 3 6 7 4 => 8 1 2 3 7 9 4

or

8 1 7 => 8 1 7 correct

Can you post exactly what you do, what should happen and what happens instead for a single, minimal example?

[matony1989]

Hello.

Those numbers are color indexes as listed above. So it is like this Player X, Y(color). In my case, I only used position of color index to determine player position and amount of players (amount of colors)

player 1 - 8 (white), player 2 - 1(yellow), player 3 - 7(teal) set in LUA => 8 1 7 in game menu - correct

map 14
LUA => Game menu
player 1 8 (white) , => 8 (white)
player 2 1(yellow), => 1 (yellow)
player 3 2(red), => 2 (red)
player 4 3(pink), => 3 (pink)
player 5 6(orange), => 7 (teal)
player 6 7(teal) = > 9 (brown)
player 7 4(grey) => 4(grey)

VS what was in LUA:

and if I change order, the colors are correct.

Hope this clears it up a bit

[Flamefire]

Ok that clears it up and I'm quite sure I know the cause:

s25client/libs/s25main/network/GameServer.cpp

Lines 1587 to 1620 in dc0ce04

	void GameServer::CheckAndSetColor(unsigned playerIdx, unsigned newColor)
	{
	RTTR_Assert(playerIdx < playerInfos.size());
	RTTR_Assert(playerInfos.size() <= PLAYER_COLORS.size()); // Else we may not find a valid color!
	JoinPlayerInfo& player = playerInfos[playerIdx];
	RTTR_Assert(player.isUsed()); // Should only set colors for taken spots
	// Get colors used by other players
	std::set<unsigned> takenColors;
	for(unsigned p = 0; p < playerInfos.size(); ++p)
	{
	// Skip self
	if(p == playerIdx)
	continue;
	JoinPlayerInfo& otherPlayer = playerInfos[p];
	if(otherPlayer.isUsed())
	takenColors.insert(otherPlayer.color);
	}
	// Look for a unique color
	int newColorIdx = JoinPlayerInfo::GetColorIdx(newColor);
	while(helpers::contains(takenColors, newColor))
	newColor = PLAYER_COLORS[(++newColorIdx) % PLAYER_COLORS.size()];
	if(player.color == newColor)
	return;
	player.color = newColor;
	SendToAll(GameMessage_Player_Color(playerIdx, player.color));
	LOG.writeToFile("SERVER >>> BROADCAST: NMS_PLAYER_TOGGLECOLOR(%d, %d)\n") % playerIdx % player.color;
	}

This code will check, if the color is already taken and select the next free one. So the GetPlayer(1):SetColor(2) will choose will use color 3 because 2 is still taken by GetPlayer(2). So if you change player 2 before 1 it will work.

I'm not sure how to handle it automatically: When Luas SetColor is run, the previous color change will/might not be completed so it won't be able to detect if a previous change "freed up" a color for use.
Maybe we can add a force flag to the network protocol so it will change the other players color instead when the current is taken. This would only be used by Lua.

@Flow86 Ideas?

[Spikeone]

Doesn't that mean, that if you set colors in order (e.g. first color 0, then 1, then 2) ignoring the player order should work? Asked @matony1989 to try that but it didn't work.

[matony1989]

@Spikeone it worked. What didn´t work was moving the code block after setting players.

[Flamefire]

Doesn't that mean, that if you set colors in order (e.g. first color 0, then 1, then 2) ignoring the player order should work?

What do you mean by that? IIRC the player colors are set in order by default (player idx = color idx) So if you set color 0 to any player but 0 it will set it to another color as 0 is taken by player 0.

@Spikeone it worked. What didn´t work was moving the code block after setting players.

What exactly did you do?

[matony1989]

I only moved the SetColor part within function onSettingsReady() at the very end, after

for p = 1, rttr:GetNumPlayers() - 1 do
        rttr:GetPlayer(p):SetTeam(TM_TEAM1)
        rttr:GetPlayer(p):SetAI(3)
        rttr:GetPlayer(p):SetName(_("name"))
    end

It was written before this block.

[Spikeone]

Doesn't that mean, that if you set colors in order (e.g. first color 0, then 1, then 2) ignoring the player order should work?

What do you mean by that? IIRC the player colors are set in order by default (player idx = color idx) So if you set color 0 to any player but 0 it will set it to another color as 0 is taken by player 0.

@Spikeone it worked. What didn´t work was moving the code block after setting players.

What exactly did you do?

Ah I thought you mean, that when a player "loses" his color index ins increased by 1, thus I thought the following could work:

rttr:GetPlayer(5):SetColor(0)
    rttr:GetPlayer(2):SetColor(2)
    rttr:GetPlayer(7):SetColor(5)
    rttr:GetPlayer(6):SetColor(6)
    rttr:GetPlayer(3):SetColor(7)
    rttr:GetPlayer(0):SetColor(8)
    rttr:GetPlayer(4):SetColor(9)
    rttr:GetPlayer(1):SetColor(10)

[Flamefire]

I only moved the SetColor part within function onSettingsReady() at the very end, after

I see. Colors are assigned when a slot gets taken, so setting colors before avoids the check as the slot isn't taken yet.

So this works:

rttr:GetPlayer(0):SetColor(5)
rttr:GetPlayer(5):SetAI(3)
rttr:GetPlayer(5):SetColor(0)

This does not:

rttr:GetPlayer(5):SetAI(3)
rttr:GetPlayer(0):SetColor(5)  -- Taken by player 5 -> Will be 8 (next after 7)
rttr:GetPlayer(5):SetColor(0)  -- Works as color 0 unused after above

@matony1989 It sounds like you did

rttr:GetPlayer(0):SetColor(5)
rttr:GetPlayer(5):SetColor(0)

rttr:GetPlayer(5):SetAI(3)

Is this the case? Because this is actually supposed to fail (unused players shouldn't have properties set)

[matony1989]

I did try it like this:

function onSettingsReady()
    checkVersion()
    rttr:Log("-----------------------\n Mission Script loaded... \n-----------------------\n")

    rttr:ResetAddons()
    rttr:SetAddon(ADDON_FRONTIER_DISTANCE_REACHABLE, true)
    rttr:SetAddon(ADDON_CATAPULT_GRAPHICS, true)
    rttr:SetAddon(ADDON_NUM_SCOUTS_EXPLORATION, 4)
    rttr:SetGameSettings({
        ["fow"] = EXP_CLASSIC,
        ["teamView"] = false,
        ["lockedTeams"] = true,
        ["startWares"] = SWR_NORMAL,
        ["objective"] = GO_TOTALDOMINATION
    })

    rttr:GetPlayer(0):SetNation(NAT_VIKINGS)
    rttr:GetPlayer(1):SetNation(NAT_JAPANESE)
    rttr:GetPlayer(2):SetNation(NAT_BABYLONIANS)
    rttr:GetPlayer(3):SetNation(NAT_AFRICANS)
    rttr:GetPlayer(4):SetNation(NAT_ROMANS)
    rttr:GetPlayer(5):SetNation(NAT_BABYLONIANS)
    rttr:GetPlayer(6):SetNation(NAT_ROMANS)

    rttr:GetPlayer(0):SetPortrait(4)
    rttr:GetPlayer(1):SetPortrait(7)
    rttr:GetPlayer(2):SetPortrait(12)
    rttr:GetPlayer(3):SetPortrait(9)
    rttr:GetPlayer(4):SetPortrait(1)
    rttr:GetPlayer(5):SetPortrait(12)
    rttr:GetPlayer(6):SetPortrait(2)

    rttr:GetPlayer(0):SetColor(2)
    rttr:GetPlayer(1):SetColor(3)
    rttr:GetPlayer(2):SetColor(4)
    rttr:GetPlayer(3):SetColor(5)
    rttr:GetPlayer(4):SetColor(6)
    rttr:GetPlayer(5):SetColor(7)
    rttr:GetPlayer(6):SetColor(8)

    for p = 1, rttr:GetNumPlayers() - 1 do
        rttr:GetPlayer(p):SetTeam(TM_TEAM1)
        rttr:GetPlayer(p):SetAI(3)
        rttr:GetPlayer(p):SetName(_("name"))
    end
end

vs

function onSettingsReady()
    checkVersion()
    rttr:Log("-----------------------\n Mission Script loaded... \n-----------------------\n")

    rttr:ResetAddons()
    rttr:SetAddon(ADDON_FRONTIER_DISTANCE_REACHABLE, true)
    rttr:SetAddon(ADDON_CATAPULT_GRAPHICS, true)
    rttr:SetAddon(ADDON_NUM_SCOUTS_EXPLORATION, 4)
    rttr:SetGameSettings({
        ["fow"] = EXP_CLASSIC,
        ["teamView"] = false,
        ["lockedTeams"] = true,
        ["startWares"] = SWR_NORMAL,
        ["objective"] = GO_TOTALDOMINATION
    })

    rttr:GetPlayer(0):SetNation(NAT_VIKINGS)
    rttr:GetPlayer(1):SetNation(NAT_JAPANESE)
    rttr:GetPlayer(2):SetNation(NAT_BABYLONIANS)
    rttr:GetPlayer(3):SetNation(NAT_AFRICANS)
    rttr:GetPlayer(4):SetNation(NAT_ROMANS)
    rttr:GetPlayer(5):SetNation(NAT_BABYLONIANS)
    rttr:GetPlayer(6):SetNation(NAT_ROMANS)

    rttr:GetPlayer(0):SetPortrait(4)
    rttr:GetPlayer(1):SetPortrait(7)
    rttr:GetPlayer(2):SetPortrait(12)
    rttr:GetPlayer(3):SetPortrait(9)
    rttr:GetPlayer(4):SetPortrait(1)
    rttr:GetPlayer(5):SetPortrait(12)
    rttr:GetPlayer(6):SetPortrait(2)

    for p = 1, rttr:GetNumPlayers() - 1 do
        rttr:GetPlayer(p):SetTeam(TM_TEAM1)
        rttr:GetPlayer(p):SetAI(3)
        rttr:GetPlayer(p):SetName(_("name"))
    end

    rttr:GetPlayer(0):SetColor(2)
    rttr:GetPlayer(1):SetColor(3)
    rttr:GetPlayer(2):SetColor(4)
    rttr:GetPlayer(3):SetColor(5)
    rttr:GetPlayer(4):SetColor(6)
    rttr:GetPlayer(5):SetColor(7)
    rttr:GetPlayer(6):SetColor(8)

end

@Spikeone I tried what you asked again. If there is offset of player and color index ( P(0) = C(2) ) it breaks. I originally assigned colors like player indexes P(0) = C(0)

It was supposed to be :
red
pink
grey
green
orange
teal
white

[Spikeone]

@Spikeone I tried what you asked again. If there is offset of player and color index ( P(0) = C(2) ) it breaks. I originally assigned colors like player indexes P(0) = C(0)

If I understand @Flamefire correctly, the colors need to be set inside the loop:

for p = 1, rttr:GetNumPlayers() - 1 do
        rttr:GetPlayer(p):SetTeam(TM_TEAM1)
        rttr:GetPlayer(p):SetAI(3)
        rttr:GetPlayer(0):SetColor(p + 2)
        rttr:GetPlayer(p):SetName(_("name"))
end

[Flamefire]

What I expect:

    rttr:GetPlayer(0):SetColor(2) -- 7
    rttr:GetPlayer(1):SetColor(3) -- 8
    rttr:GetPlayer(2):SetColor(4) -- 9
    rttr:GetPlayer(3):SetColor(5) -- 10
    rttr:GetPlayer(4):SetColor(6) -- 0
    rttr:GetPlayer(5):SetColor(7) -- 1
    rttr:GetPlayer(6):SetColor(8) -- 2

This seems to match the screenshot.

P(i) = C(i) is the default assignment, so no change

If I understand @Flamefire correctly, the colors need to be set inside the loop:

That kinda works because you set the color higher so that slot isn't used yet.

[matony1989]

@Spikeone I tried what you asked again. If there is offset of player and color index ( P(0) = C(2) ) it breaks. I originally assigned colors like player indexes P(0) = C(0)

If I understand @Flamefire correctly, the colors need to be set inside the loop:

for p = 1, rttr:GetNumPlayers() - 1 do
        rttr:GetPlayer(p):SetTeam(TM_TEAM1)
        rttr:GetPlayer(p):SetAI(3)
        rttr:GetPlayer(0):SetColor(p + 2)
        rttr:GetPlayer(p):SetName(_("name"))
end

I can later test this for map 14. This will not work for other maps, since the position of "characters" is different on each map. I need to ask creator of map if he thought about it, or if I can change order of characters to avoid this color issue. OR change order in editor, to avoid the color issue too,