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

Change MIX files to be loaded from a single list #150

Merged
merged 5 commits into from
May 9, 2024

Conversation

ZivDero
Copy link
Contributor

@ZivDero ZivDero commented May 3, 2024

Changes mix files to be loaded from one list with the value controlling if they are required or optional. Meant to help merge the 2 TS branches.

Config for YR:

0=langmd.mix,1
1=language.mix,1
2=$EXPANDMD
3=ra2md.mix,1
4=ra2.mix,1
5=cachemd.mix,1
6=cache.mix,1
7=localmd.mix,1
8=local.mix,1
9=$RA2ECACHE
10=$RA2ELOCAL
11=conqmd.mix,1
12=genermd.mix,1
13=generic.mix,1
14=isogenmd.mix,1
15=isogen.mix,1
16=conquer.mix,1
17=marble.mix,0		; borrowed from Final Alert 2

@ZivDero ZivDero force-pushed the feature/one-mix-list branch 3 times, most recently from 94dae4c to 455874a Compare May 3, 2024 20:49
Copy link
Owner

@Rampastring Rampastring left a comment

Choose a reason for hiding this comment

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

This looks somehow heavy to parse for my eyes...

I'd prefer something like

0=landmd.mix,1
1=languagemd.mix,1
2=$EXPANDMD
; ...

Where the optional number after the name would denote whether it's required

Maybe even the current style would be good enough if the "required" status could be specified with a boolean or a number though

langmd.mix=1
language.mix=1
$EXPANDMD=0

Comment on lines 38 to 60

var mixesSection = iniFile.GetSection("MIXFiles");
foreach (var kvp in mixesSection.Keys)
{
var parts = kvp.Value.Split(',', StringSplitOptions.TrimEntries);
string mixName = parts[0];

if (IsSpecialMixName(mixName))
{
HandleSpecialMixName(mixName);
continue;
}

bool isRequired = false;

if (parts.Length > 1)
isRequired = Conversions.BooleanFromString(parts[1], isRequired);

if (isRequired)
LoadRequiredMixFile(mixName);
else
LoadOptionalMixFile(mixName);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Make this part into its own function, such as LoadMixFiles, to keep the general ReadConfig function flow clean. I liked how it was originally just a few DoForEveryValueInSection calls.

It could even be LoadMixFile(string entry) so it could be called with iniFile.DoForEveryValueInSection.

@ZivDero ZivDero requested a review from Rampastring May 9, 2024 15:28
@Rampastring Rampastring merged commit 2a80e3d into Rampastring:master May 9, 2024
1 check passed
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.

None yet

2 participants