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

Add new mtaserver.conf rules for server browser/list #3761

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

Fernando-A-Rocha
Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha commented Oct 1, 2024

In preparation of #759, #998 and #2104

MTA servers use ASE protocol to respond with server information (name, player count, max players, server rules, etc).
In particular, server rules are key-value pairs that can be set using setRuleValue. These are already provided in servers' ASE responses.

https://int64.org/docs/gamestat-protocols/ase.html
image

My idea was to allow definition of key-value rule pairs in the mtaserver.conf file.

  • I have added the follow <rule> entries:
<!-- These rules are optional parameters for the server browser/list (they can be left empty).
     Each rule has their own requirements. Read carefully. -->

<!-- A brief description of the server. -->
<rule name="description" value="" />

<!-- A comma separated list of languages that the server supports (e.g. en). -->
<rule name="languages" value="" />

<!-- A comma separated list of tags that describe the server (e.g. freeroam,roleplay). -->
<rule name="tags" value="" />

<!-- The URL of the server's website. -->
<rule name="website_url" value="" />

<!-- A social media URL #1 (e.g. Discord). -->
<rule name="social1_url" value="" />

<!-- A social media URL #2 (e.g. YouTube). -->
<rule name="social2_url" value="" />

<!-- A social media URL #3 (e.g. Facebook). -->
<rule name="social3_url" value="" />
  • I also made the server load these entries from the mtaserver.conf, setting their values on startup in the ASE class. It doesn't validate any of the data (as setRuleValue doesn't), so server developers are free to define the rules they want.

Then the server browser should read these values and act accordingly, thanks to a UI revamp (FUTURE - Another PR).

Pros of this method:

  • Doesn't require changing existing supported ASE protocol used by many services external to MTA
  • Server rules can already be altered using Lua functions
  • The main worry will be modifying the server browser (UI) to interpret these new values

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 1, 2024

As I showed on Discord, example output of a server's ASE using https://github.com/opengsq/opengsq-python that @botder suggested :

opengsq ase --host 127.0.0.1 --port 22126
{"game_name": "mta", "game_port": 22003, "hostname": "Default MTA Server", "game_type": "play", "map": "None", "version": "1.6", "password": false, "num_players": 0, "max_players": 32, "rules": {"languages": "en", "tags": "roleplay,rp,serious", "logo_url": "https://www.example.com/logo.png"}, "players": []}

We can set any key-value pair information, and work with this.

@CrosRoad95
Copy link
Contributor

Add social media?

<rule name="discord" value="discord.gg/something" />
<rule name="facebook" value="facebook.com/something" />

url should be a resource on this server only, therefore you put foo/logo.png it should try to download it from server http port., from resource "foo" file "logo.png".

If possible, add size of potentially downloaded files.

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 1, 2024

Social media links are a great addition!

I don't think that in a first version, we will have servers serving the media files themselves. This is plenty of work imo 😅 External urls is an alright solution, and it could accept only upload.mtasa.com for now

@CrosRoad95
Copy link
Contributor

Take an inspiration from nanos world server browser:
image

@TheNormalnij TheNormalnij added the enhancement New feature or request label Oct 1, 2024
@T-MaxWiese-T
Copy link

The number of resources and names are displayed in the FiveM server list. I wouldn't consider this useful for MTA players, but it could be used for statistics and for example resource authors could see how often their resources are used.
But what I would find much more interesting would be the download size of the resources, that would be a gamechanger. Players could see how big the download is and then estimate how long it takes, how much disk space it would take up to see if it's worth it. This could possibly also lead to the servers paying more attention to using resources more efficiently.

@Fernando-A-Rocha
Copy link
Contributor Author

I absolutely agree with showing the download size of the resources. I imagine it's a value that's already calculated by the server, it would be a matter of announcing it to the server browser

@ffsPLASMA
Copy link
Contributor

The number of resources and names are displayed in the FiveM server list. I wouldn't consider this useful for MTA players, but it could be used for statistics and for example resource authors could see how often their resources are used. But what I would find much more interesting would be the download size of the resources, that would be a gamechanger. Players could see how big the download is and then estimate how long it takes, how much disk space it would take up to see if it's worth it. This could possibly also lead to the servers paying more attention to using resources more efficiently.

Resource display would have to to be filtered by resource type though. FFS got over 20k map resources. This will be an issue if you try to display them all :mreow:

@Fernando-A-Rocha
Copy link
Contributor Author

The number of resources and names are displayed in the FiveM server list. I wouldn't consider this useful for MTA players, but it could be used for statistics and for example resource authors could see how often their resources are used. But what I would find much more interesting would be the download size of the resources, that would be a gamechanger. Players could see how big the download is and then estimate how long it takes, how much disk space it would take up to see if it's worth it. This could possibly also lead to the servers paying more attention to using resources more efficiently.

Resource display would have to to be filtered by resource type though. FFS got over 20k map resources. This will be an issue if you try to display them all :mreow:

Are all those 20k resources with state "loaded"? 😳

@ffsPLASMA
Copy link
Contributor

ffsPLASMA commented Oct 2, 2024

The number of resources and names are displayed in the FiveM server list. I wouldn't consider this useful for MTA players, but it could be used for statistics and for example resource authors could see how often their resources are used. But what I would find much more interesting would be the download size of the resources, that would be a gamechanger. Players could see how big the download is and then estimate how long it takes, how much disk space it would take up to see if it's worth it. This could possibly also lead to the servers paying more attention to using resources more efficiently.

Resource display would have to to be filtered by resource type though. FFS got over 20k map resources. This will be an issue if you try to display them all :mreow:

Are all those 20k resources with state "loaded"? 😳

loaded, not running.

@Fernando-A-Rocha
Copy link
Contributor Author

The server probably should only make a list of the resources that are running, for such a feature that would show them in the server browser.

@T-MaxWiese-T
Copy link

The server probably should only make a list of the resources that are running, for such a feature that would show them in the server browser.

I think if you want to have this with the number of resources and names you should do it separately. I guess for the server browser it has no real relevance but only for statistical purposes.

@Fernando-A-Rocha
Copy link
Contributor Author

Added social urls, removed icon/banner image URLs for now. This is not a necessery feature yet

@T-MaxWiese-T
Copy link

Added social urls, removed icon/banner image URLs for now. This is not a necessery feature yet

Is Facebook really still relevant today?

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 3, 2024

Haha who cares, the Facebook comment in "e.g." is just a suggestion. I added 3 social URLs that can be filled with any URLs that servers desire, MTA server browser could allow only certain domains (e.g. discord, youtube, twitch, vk.com, etc). My idea is to draw these Social URLs in a panel that displays server info when the server is selected.

@Fernando-A-Rocha
Copy link
Contributor Author

Reminder that setRuleValue gives servers the freedom of setting any values for any keys, without any validation. It can remain like that. The server browser UI will be responsible for the sanitization of data, and choosing what to display in the client.

@T-MaxWiese-T
Copy link

Perhaps it should be documented how many characters may be used for each rule. I think this would be particularly relevant for "description" and "tags". So that as much data as possible arrives and so that everything is visible in the GUI. But how much space the GUI will have depends on the implementation.

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 3, 2024

The current implementation in c++ (setRuleValue) limits key/value strings to 200 characters:

#define MAX_RULE_VALUE_LENGTH 200

I have documented this limit in the wiki now

@Fernando-A-Rocha Fernando-A-Rocha marked this pull request as ready for review October 3, 2024 12:50
@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 3, 2024

I made the server load the rules defined in mtaserver.conf into the ASE, applying the same method that Lua function setRuleValue uses.

Result successful:

opengsq ase --host 127.0.0.1 --port 22126
{"game_name": "mta", "game_port": 22003, "hostname": "Default MTA Server", "game_type": "play", "map": "None", "version": "1.6n", "password": false, "num_players": 0, "max_players": 32, "rules": {"description": "This is my awesome test server", "languages": "pt-br,en", "social1_url": "https://discord.gg/mtasa", "tags": "roleplay,serious,textrp", "website_url": "https://example.com"}, "players": []}

The rules applied are returned in the 22126 udp result

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 3, 2024

Accepting reviews, as I've written c++ code to make the rules load into the ASE on startup, concluding this feature IMO

Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CMainConfig.h Outdated Show resolved Hide resolved
@Fernando-A-Rocha
Copy link
Contributor Author

All ok?

Server/mods/deathmatch/logic/CMainConfig.cpp Outdated Show resolved Hide resolved
@Fernando-A-Rocha
Copy link
Contributor Author

All ok

@Fernando-A-Rocha
Copy link
Contributor Author

Review requested once again as I've edited CMainConfig::AddMissingSettings() to support XML nodes with attributes. The script was not taking any attributes into account, not being appropriate for multiple rule name="abc" value="abc" entries.

Comment on lines 886 to 906
if (tempNode->GetTagName() == templateNodeTagName)
{
bool bAttributesMatch = true;
CXMLAttributes& attributes = tempNode->GetAttributes();
for (auto it3 = templateAttributes.ListBegin(); it3 != templateAttributes.ListEnd(); ++it3)
{
CXMLAttribute* templateAttribute = *it3;
const SString& strKey = templateAttribute->GetName();
const SString& strValue = templateAttribute->GetValue();
CXMLAttribute* foundAttribute = attributes.Find(strKey);
if (!foundAttribute || foundAttribute->GetValue() != strValue)
{
bAttributesMatch = false;
break;
}
}
if (bAttributesMatch)
{
foundNode = tempNode;
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

use early continue

Copy link
Member

Choose a reason for hiding this comment

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

There is no early continue practice. Leave previous variant.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no early continue practice. Leave previous variant.

Yes there is.

Copy link
Member

Choose a reason for hiding this comment

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

Yes there is.

Prove it

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It's early-returns and not early continue

My friend, look below that

Copy link
Member

@TheNormalnij TheNormalnij Oct 5, 2024

Choose a reason for hiding this comment

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

It's your own rule, lol (added 3 weeks ago)

Copy link
Contributor

@TracerDS TracerDS Oct 5, 2024

Choose a reason for hiding this comment

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

It's your own rule, lol

Its been merged nonetheless. If you have any issues, please post them at mtasa-docs repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It changes almost nothing in terms of readability tbh. But ye it was approved

@CrosRoad95
Copy link
Contributor

In this pull request you just extending what ase is returning, visual aspects in main menu to display it will be in another pull request?

@Fernando-A-Rocha
Copy link
Contributor Author

In this pull request you just extending what ase is returning, visual aspects in main menu to display it will be in another pull request?

I don't even extend what ase is returning. This pr just adds a feature to mtaserver.conf for setting "rules" on startup (like using setRuleValue) which are settings for the server browser (old feature unused).

Visual edits to the server browser to start using these "rules" will be made later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants