Skip to content

Allow changing color of unused player slots #1885

Draft
Flamefire wants to merge 3 commits intoReturn-To-The-Roots:masterfrom
Flamefire:colors
Draft

Allow changing color of unused player slots #1885
Flamefire wants to merge 3 commits intoReturn-To-The-Roots:masterfrom
Flamefire:colors

Conversation

@Flamefire
Copy link
Member

@Flamefire Flamefire commented Feb 3, 2026

This is useful for use by LUA to set player colors before filling the
slots avoiding temporary conflicts (same colors).

See #1884

Mention this in the docs which were wrong in this regard

Edit: doesn't work as in single player games all slots are filled by AI

This can be used to  `static_assert` the size of the `PLAYER_COLORS` array.
This is useful for use by LUA to set player colors before filling the
slots avoiding temporary conflicts (same colors).

See Return-To-The-Roots#1884
@Flamefire Flamefire requested review from Flow86 and Spikeone February 3, 2026 13:35
@Flamefire Flamefire marked this pull request as draft February 3, 2026 19:44
@Flow86
Copy link
Member

Flow86 commented Feb 3, 2026

can we maybe change/extend the lua code so you have to specify all colors at once, e.g to set all colors at once?

@Flamefire
Copy link
Member Author

That would still require adapting the protocol as it is the server that sets the colors.
So my best idea is to have a "force" parameter for the protocol that changes the other conflicting color instead of the current/new one.

This way you get exactly what you want when you do GetPlayer:SetColor for this player, but if you don't set the other player you'll get a different one there.

Or we remove that part checking colors on set: We have a readiness check that verifies unique colors: https://github.com/Flamefire/s25client/blob/fb01a4f33a076b7fd63b174d0ac7bde0bb176544/libs/s25main/network/GameServer.cpp#L1446-L1458

With that we could check the colors only before starting. Given the original documentation of the Lua function we might have even intended to allow 2 players with the same color, at least for scripted maps

@Spikeone
Copy link
Member

Spikeone commented Feb 4, 2026

With that we could check the colors only before starting. Given the original documentation of the Lua function we might have even intended to allow 2 players with the same color, at least for scripted maps

Well, this does sound great, since you then could place 2 HQs somewhere and make it seem as if they are the same player, or just make alle enemies red.

On the other hand, this might lead to unexpected results when inexperienced users try to write LUA, but I think with proper documentation, this should be fine.

@Flamefire
Copy link
Member Author

On the other hand, this might lead to unexpected results when inexperienced users try to write LUA, but I think with proper documentation, this should be fine.

I mean that is what is documented right now so making sure it works would be a bugfix.

When using the color toggle manually it picks a free color. There is only a small chance when 2 people change their color at the "same" time to have duplicates.
So shall we check at all for duplicates, only when setting and/or allow when set by Lua?
I would go the easy route and accept any color requested by a client/player but make the UI select the next free color when clicked, which might be duplicate if another player is just changing to this color too

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