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

Use string for faction in game launch message #834

Open
Sheikah45 opened this issue Sep 13, 2021 · 6 comments · May be fixed by #835
Open

Use string for faction in game launch message #834

Sheikah45 opened this issue Sep 13, 2021 · 6 comments · May be fixed by #835
Milestone

Comments

@Sheikah45
Copy link
Member

Sheikah45 commented Sep 13, 2021

It would be nice to consistently use the string version of faction in all the messages from the server. All clients going back to 1.4.6 are able to handle the faction as a string.

1.4.6 is the current minimum version of the client

@BlackYps BlackYps linked a pull request Sep 13, 2021 that will close this issue
@Askaholic Askaholic added this to the v2.0 milestone Sep 13, 2021
@Sheikah45
Copy link
Member Author

I don't know if this needs to wait until v2.0 since all clients are designed to be able to use a string or an int as the faction serialization. So it is not a breaking change.

@Askaholic
Copy link
Collaborator

It’s still a breaking API change. I’ve written a bit about this topic here: https://github.com/FAForever/server/blob/develop/CONTRIBUTING.md#version-numbers

@BlackYps
Copy link
Collaborator

Does that mean that client version 1.6 has to wait for server v2.0?

@Askaholic
Copy link
Collaborator

No? The client should not depend on a breaking change existing, especially when that change isn’t merged.

@BlackYps
Copy link
Collaborator

Seems like Sheikah has some work to do to fix the release candidate then xD

@Sheikah45
Copy link
Member Author

I actually already have a pr that fixes it. Just requested this on principle XP

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 a pull request may close this issue.

3 participants