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

Added Tags Feature to the BeamMP Server. #192

Merged
merged 5 commits into from
Dec 28, 2023
Merged

Added Tags Feature to the BeamMP Server. #192

merged 5 commits into from
Dec 28, 2023

Conversation

Starystars67
Copy link
Member

This PR adds a tags functionality to the BeamMP server config and heartbeat.
The intended purpose of this is to enable better filtering on the server list based on keywords such as gamemode or roleplay.

@lionkor
Copy link
Member

lionkor commented Sep 29, 2023

TOML has arrays, so this should probably be an array:

Tags = ["Tag1", "Tag2"]

@Starystars67
Copy link
Member Author

Hi Lion,

I have been thinking this one over over the last week and bit and I am thinking that though for us more technically capable people an array is the correct approach.

BeamMP however is also used my people who are not as technically capable and I worry that a potential missing quote mark or comma in an array could cause the config to not be loaded and then raise new posts in support.

Happy to hear other sides on this though.

@lionkor
Copy link
Member

lionkor commented Oct 10, 2023

Fair enough, it's definitely a more complicated syntax as a TOML array. What bothers me about this approach is that it's an undocumented syntax, the parser for which will be in the backend. In this case, the comment should probably explain the syntax, something like "Separate tags with commas, like so: Tag1,Tag2,Tag3" or something.
And then any tool parsing or writing to the config will have to also write a parser/serializer for it, so either way would be good to document it in the file itself.

Other than that, maybe the team can decide on some good default tags, because I can't think of any.

As for the windows compile error, that maybe a dependency issue, Anon might be able to help there.

@Starystars67
Copy link
Member Author

Fair enough, it's definitely a more complicated syntax as a TOML array. What bothers me about this approach is that it's an undocumented syntax, the parser for which will be in the backend. In this case, the comment should probably explain the syntax, something like "Separate tags with commas, like so: Tag1,Tag2,Tag3" or something. And then any tool parsing or writing to the config will have to also write a parser/serializer for it, so either way would be good to document it in the file itself.

Yep, that is what I did on line 106 in src/TConfig.cpp.

Other than that, maybe the team can decide on some good default tags, because I can't think of any.

Indeed.

As for the windows compile error, that maybe a dependency issue, Anon might be able to help there.

Will do. @Anonymous-275 :P

@lionkor
Copy link
Member

lionkor commented Dec 10, 2023

Needs better or no default tags

@lionkor lionkor added the feature New feature or request label Dec 10, 2023
@Starystars67
Copy link
Member Author

Needs better or no default tags

Done. Changed the default to Freeroam to match the majority of servers presently on the list.

@Starystars67
Copy link
Member Author

Starystars67 commented Dec 28, 2023

Backend support for this is already implemented.

@lionkor
Copy link
Member

lionkor commented Dec 28, 2023

Blocked by #227

@lionkor
Copy link
Member

lionkor commented Dec 28, 2023

Made the tags pretty-print in the debug view

image

image

@lionkor lionkor merged commit 810788a into master Dec 28, 2023
3 checks passed
@lionkor lionkor deleted the feature-tags branch December 28, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants