djhackersdev / bemanitools

Runs recent Konami arcade games and emulates various arcade hardware.
The Unlicense
87 stars 17 forks source link

launcher: allow overriding service url and url slash from the command line - [merged] #172

Closed icex2 closed 1 year ago

icex2 commented 3 years ago

In GitLab by @xyen on Dec 21, 2020, 08:46

_Merges ea3override -> master

This change allows users to override the service URL and slash from the command line, so that they don't need to touch ea3-config.xml in some scenarios.

Also fixes a mistake where the jbhook was using the wrong type enum (resulting in an incorrect comment).

This addresses #63

icex2 commented 3 years ago

Not: Would call these ea3_ident_set_property_xxx as they not just replace existing nodes but set non existing ones

icex2 commented 3 years ago

You can reduce the number of options by using a NULL check on the string to check if override is enabled.

icex2 commented 3 years ago

Duplicate? Isn’t override_urlslash and override_urlslash_enable doing the same?

icex2 commented 3 years ago

In GitLab by @xyen on Dec 21, 2020, 18:53

Commented on src/main/launcher/options.h line 23

No, because override_urlslash_enable sets the value to override_urlslash which is either true, or false, and the value inside of the original ea3-config could be either.

icex2 commented 3 years ago

In GitLab by @xyen on Dec 21, 2020, 18:53

Commented on src/main/launcher/ea3-config.h line 41

True, I suppose, but their usage in this code is only for replacing existing nodes.

icex2 commented 3 years ago

Still, different expactation of function naming compared to its implementation. Nothing severe, so I can let it pass.

icex2 commented 3 years ago

resolved all threads

icex2 commented 3 years ago

For consistency, I'd rather leave it explicit.

What consistency are you referring to? I don't see another case in launcher that is doing something similar.

icex2 commented 3 years ago

In GitLab by @xyen on Dec 21, 2020, 19:04

Commented on src/main/launcher/options.c line 144

changed this line in version 2 of the diff

icex2 commented 3 years ago

In GitLab by @xyen on Dec 21, 2020, 19:04

added 7 commits

Compare with previous version

icex2 commented 3 years ago

In GitLab by @xyen on Dec 21, 2020, 19:05

resolved all threads

icex2 commented 3 years ago

approved this merge request