Check for explicitly set logger config before overwriting it

[niklaswimmer]

This change allows users to utilize the log4j2.configurationFile system property for configuring the logging context to their liking.

This property is read by log4j2 to find the configuration file it should load during initialization. Previously, setting this property only affected logging output during the boot stage (anything before launching Minecraft itself) due to the unconditional logger context reconfiguration.

Tying this logic to the already existing system property used by log4j2 seemed the most logical, considering that this property is the only approachable way for specifying an alternative configuration in the first place.

[Technici4n]

That seems reasonable. @FiniteReality you also worked on this area, what do you think?

[marchermans]

Does this then use the vanilla logging configuration needed for the launcher to stop complaining? (If we hosted and provided one?)

[Technici4n]

What are these launcher complaints? I never realized it was complaining.

[marchermans]

What are these launcher complaints? I never realized it was complaining.

Look at the first line that the launcher outputs. It states that the logging configuration is missing and that it assumes plain text.

[niklaswimmer]

Does this then use the vanilla logging configuration needed for the launcher to stop complaining?

I am not sure I understand this question. The log4j2.configurationFile property expects an URL and loads the config file from there. All other configs are ignored. This change does not affect the current logging configuration and its output.

See here and here.

[sciwhiz12]

This looks fine to me.

The topic of the vanilla launcher not being satisfied because of our differing logging configuration is for another issue/PR, since we really haven't digged into that topic well enough to tell what the vanilla launcher is looking for (such that it can't find it and therefore defaults to assuming plain-text), and what the proper solution is (if it entails modifying our logging configuration, or otherwise).

This PR is just so users (mod developers and perhaps end users?) can modify Log4J as needed through a configuration file, since the current way just reforcibly reconfigures Log4J using FML's logging configuration without exception.

[sciwhiz12]

I've just been informed, and I apologize I didn't catch this, that we didn't account for the fact that the vanilla Minecraft launcher supplies the logging configuration through the same-named system property, so at the moment (if we update FML in NeoForge without any other changes, and barring any other factors I may not have considered) vanilla's configuration would always override ours (as the system property would always be present in production).

We have a couple of options, I think:

1. Revert this PR. (I'd consider this a bit of a last resort, personally.)

2. Modify the installed profile to insert our own log4j2.configurationFile system property to hopefully override vanilla's with our own (while also allowing users to override ours).

3. Investigate modifying the installed profile to supply our logging configuration file through the logging object (as seen in the game manifest).

4. Fast-track Change how logging works to make configuration easier McModLauncher/modlauncher#119 to allow us to specify additional logging configuration files independent of the system property, with proper aggregation (instead of Log4J's winner-takes-all method).

[FiniteReality]

Modify the installed profile to insert our own log4j2.configurationFile system property to hopefully override vanilla's with our own (while also allowing users to override ours).

This is only partially possible; system properties specified on the command line get overwritten by ones later in the parameter list. The only way to compose them is to separate with commas, like -Dlog4j2.configurationFile=a.xml,b.xml,c.xml - this is why in my PR I added an additional --loggingConfig argument into ModLauncher so that logging config files could be added in series

Investigate modifying the installed profile to supply our logging configuration file through the logging object (as seen in the game manifest).

This would be nice in addition to my proposed changes. Specifying it early on in the commandline args means we have a sensible default that's easy for users and modders to override.

[niklaswimmer]

I am sorry, I wasn't aware that the Vanilla Laincher uses the same system property. I only looked at this from the perspective of a mod developer and only tested the change in a dev environment.

If I knew about this problem, I would have used another system property for this check. Something like fml.logging.noOverwrite.

[Technici4n]

See #75 for a potential solution.