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

Rework server.ini parsing to not depend on the real console #1659

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

Conversation

dgelessus
Copy link
Contributor

Reworks how server.ini files are parsed, based on my console parsing refactoring in #1459. Advantages of this reworked code include:

  • Visible error messages (instead of silent failure) when something in the server.ini is obviously invalid: unknown option names, invalid integers, incorrectly formatted base-64 keys.
  • The server.ini handling is now decoupled from the console engine - only the parser is shared. This means you can no longer put console commands into the server.ini or set server.ini options from the in-game console.
  • The parsed server settings are stored in a struct and not directly into global variables. This is useful groundwork for possible future projects that need to parse more than one server.ini, such as a shard selector.

Also includes some bonus refactoring of how the Diffie-Hellman keys are stored and passed around.

Example of the new error messages:

Windows error dialog saying: "Error in server.ini file. Please check your URU installation. Line 5: Invalid key: Invalid base-64 data (did you forget to put quotes around the value?)"

The commands under the Server group are only meant for server.ini files
and aren't usable anywhere else. Similarly, server.ini files aren't
supposed to call any other console commands. This is now enforced by not
handling the server settings as real commands, but with a new dedicated
server.ini parser that handles only server settings and nothing else.

This new parser intentionally doesn't support some obscure features of
the full console, such as spaces as command group/name separators (e. g.
"Server Status" instead of "Server.Status") and console variables. This
is strictly speaking a breaking change, but unlikely to cause issues -
these features were never documented and are unused in practice.
ST::string Parse(const plFileName& fileName);
void Apply();

static ST::string Load(const plFileName& fileName);
Copy link
Member

Choose a reason for hiding this comment

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

I have a preference for returning a boolean status value and having the error string fetch being a separate call.

Copy link
Contributor Author

@dgelessus dgelessus Feb 1, 2025

Choose a reason for hiding this comment

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

Would a boolean return and an out parameter for the error message be okay too?

static bool Load(const plFileName& fileName, ST::string& errorMsg);

pfServerIni::Load is a static method and I would rather not introduce a new global "last error" variable just for this one call...

Sources/Plasma/NucleusLib/pnNetBase/pnNbConst.h Outdated Show resolved Hide resolved
Use the methods FromData_BE and FromData_LE instead, which make the
endianness explicit (or rather, more explicit than an unnamed bool
parameter with a default value).
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.

2 participants