Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

Implement #75 - Command Line Argument parsing utility and config #96

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

Conversation

Banane9
Copy link
Contributor

@Banane9 Banane9 commented Mar 30, 2023

Implements an expanded upon Version of the feature requested by @art0007i in #75.

Also changed config loading to use reflection to make it more compact, more permissive and easy to add new values.
Added some inverted properties to allow setting them with flag arguments.

@XDelta XDelta added enhancement New feature or request .NET Pull requests that update .net code labels Mar 30, 2023
@ljoonal ljoonal requested a review from zkxs March 31, 2023 13:27
Copy link
Member

@ljoonal ljoonal left a comment

Choose a reason for hiding this comment

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

I'm generally in favor of making things more generic like this would do with parsing the config, but I do question if there's not an already existing way in C# to do it easier.... At least for JSON there is, not sure about ini...

Comment on lines +30 to +60
private static readonly string[] possibleNeosExactFlagArguments =
{
"ctaa", "ctaatemporaledgepower", "ctaasharpnessenabled", "ctaaadaptivesharpness", "ForceSRAnipal",
"StereoDisplay", "MixedReality", "DirectComposition", "ExternalComposition", "create_mrc_config",
"load_mrc_config"
};

private static readonly string[] possibleNeosExactParameterArguments =
{
"TextureSizeRatio", "DataPath", "CachePath"
};

private static readonly string[] possibleNeosInfixFlagArguments =
{
"CameraBiggestGroup", "CameraTimelapse", "CameraStayBehind", "CameraStayInFront", "AnnounceHomeOnLAN"
};

private static readonly string[] possibleNeosSuffixFlagArguments =
{
"GeneratePrecache", "Verbose", "pro", "UseLocalCloud", "UseStagingCloud", "ForceRelay", "Invisible",
"ForceReticleAboveHorizon", "ForceNoVoice", "ResetDash", "Kiosk", "DontAutoOpenCloudHome", "ResetUserspace",
"ForceLANOnly", "DeleteUnsyncedCloudRecords", "ForceSyncConflictingCloudRecords", "ForceIntroTutorial",
"SkipIntroTutorial", "RepairDatabase", "UseNeosCamera", "LegacySteamVRInput", "Etee", "HideScreenReticle",
"Viveport", "DisableNativeTextureUpload"
};

private static readonly string[] possibleNeosSuffixParameterArguments =
{
"Config", "LoadAssembly", "BackgroundWorkers", "PriorityWorkers", "Bench", "Watchdog", "Bootstrap",
"OnlyHost", "Join", "Open", "OpenUnsafe", "EnableOWO"
};
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's a good idea to rely on a list of Neos arguments. They can and will be changed if Neos ever gets updated again. NML should not be concerned with these, and if a mod is concerned with one, they could easily implement their own check for it IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to do only -NML. ones first, but then I was like... it'd be kinda nice to know if an upcoming one is part of the Neos ones - and then I might as well just expose them. They're also important to know for mod creators wanting to add their own, as there's a lot of them that block any other argument ending with them or even containing them

NeosModLoader/ModLoaderConfiguration.cs Outdated Show resolved Hide resolved
var parsedValue = TypeDescriptor.GetConverter(possibleProperty.PropertyType).ConvertFromInvariantString(value);
possibleProperty.SetValue(config, parsedValue);

Logger.MsgInternal($"Loaded value for {possibleProperty.Name} from file: {parsedValue}");
Copy link
Member

Choose a reason for hiding this comment

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

I imagine this'd be rather spammy too?

Copy link
Contributor Author

@Banane9 Banane9 Mar 31, 2023

Choose a reason for hiding this comment

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

It basically lists all the values that have been set - could make this Debug, but enabling debug may only happen after this...

NeosModLoader/ModLoaderConfiguration.cs Outdated Show resolved Hide resolved
Comment on lines +20 to +36
public bool ExposeLateTypes
{
if (_configuration == null)
{
// the config file can just sit next to the dll. Simple.
string path = Path.Combine(GetAssemblyDirectory(), CONFIG_FILENAME);
_configuration = new ModLoaderConfiguration();
get => !HideLateTypes;
set => HideLateTypes = !value;
}

public bool ExposeModTypes
{
get => !HideModTypes;
set => HideModTypes = !value;
}

public bool HideConflicts
{
get => !LogConflicts;
set => LogConflicts = !value;
}
Copy link
Member

Choose a reason for hiding this comment

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

...is there an actual reason to duplicate these...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some inverted properties to allow setting them with flag arguments.

/// Contains methods to access the command line arguments Neos was launched with.
/// </summary>
/// Use analyze on Environment.GetCommandLineArgs() to find all possible arguments.
public static class LaunchArguments
Copy link
Member

Choose a reason for hiding this comment

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

Just overall to me this seems like a solution looking for a problem ¯\_(ツ)_/¯

@art0007i
Copy link
Member

maybe instead of putting all the neos default argument parsing code inside of the loader itself, it would be better to create a library mod that can be used by other mods? we have Libraries category in the manifest that seems like it would be perfect for this

@Banane9
Copy link
Contributor Author

Banane9 commented Mar 31, 2023

I'm generally in favor of making things more generic like this would do with parsing the config,

hey, doing the config parsing "manually" is @zkxs's doing. I've used an ini Parser in the SpotifyStatus server, but that's another library and also only parses it into a string dictionary style afaik

maybe instead of putting all the neos default argument parsing code inside of the loader itself, it would be better to create a library mod that can be used by other mods? we have Libraries category in the manifest that seems like it would be perfect for this

but if it's a library I can't use it for NML's config 😝

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request .NET Pull requests that update .net code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants