Skip to content

Custom resources for loading menu titles and information, as well as loading menu visuals and images#31

Closed
alfvy wants to merge 7 commits intoinfernoplus:mainfrom
alfvy:loading-menu-res
Closed

Custom resources for loading menu titles and information, as well as loading menu visuals and images#31
alfvy wants to merge 7 commits intoinfernoplus:mainfrom
alfvy:loading-menu-res

Conversation

@alfvy
Copy link
Collaborator

@alfvy alfvy commented Jan 14, 2026

these are pretty simple, since there's already ways to write custom param rows, meaning that there can be more than what came with the base game,

as well as writing to the hi/solo and low/solo files in the icon manager, but those are limited to 34 as far I know

@horfius
Copy link
Collaborator

horfius commented Jan 14, 2026

I don't really want to review this PR until your other one is merged @alfvy as it has significant overlap and is a bit confusing which pieces are new to this PR.

Copy link
Collaborator

@horfius horfius left a comment

Choose a reason for hiding this comment

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

A few cleanup bits, some clarifications, and a few dangerous code smells. Getting #28 merged first would help clarify what changes are new and which are carried over from that branch.

}

menu = LoadMsgBnd(Utility.ResourcePath(@"text\menu_dlc02.msgbnd.dcx"));
menu = LoadMsgBnd($"{Const.ELDEN_PATH}Game\\msg\\engus\\menu.msgbnd.dcx");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use Path.Combine and @.

public int AddChoice(string text)
{
foreach(FMG.Entry entry in menu[TextType.EventTextForTalk].Entries)
foreach(FMG.Entry entry in menuDlc[TextType.EventTextForTalk].Entries)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can be simplified with .First

Copy link
Collaborator

Choose a reason for hiding this comment

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

And the other functions in the file doing the same pattern should also use First instead of foreach.

{
int id = nextMenuId++;
FMG fmg = menu[TextType.GR_MenuText];
FMG fmg = menuDlc[TextType.GR_MenuText];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just do menuDlc[TextType.GR_MenuText].entries.Add(...) instead of having the fmg variable.

(var hiBxf, var lowBxf) = icon.Write();
images.Write(hiBxf, lowBxf);

hiBxf.Write($"{Const.OUTPUT_PATH}menu\\hi\\00_solo.tpfbhd", $"{Const.OUTPUT_PATH}menu\\hi\\00_solo.tpfbdt");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Path.Combine and @. With the amount that Path joins are done with OUTPUT_PATH, it might be worth creating a helper method to reduce duplication in case we need to change any formatting in the future.

var tpf = new TPF();
var texture = new TPF.Texture();

var fileName = file.Name.Split('.')[0].Split('\\')[1];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would Path.GetFileName work here or are there multiple periods before the extension? If there are multiple path separators I think this would include everything after the first one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the path pulled directly from the bxf, its not a standard path, and has some internal engine shenanigans associated

.Where(f => f.Name.ToLower().Contains("menu_load") && !f.Name.ToLower().Contains("ps5"))
.ToList();

foreach (var file in lowFiles)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this logic is exactly the same as above just with lowFiles instead of hiFiles, I suggest moving the logic to a private helper function to reduce duplication.

return (hiBxf, lowBxf);
}

public void Dispose()
Copy link
Collaborator

Choose a reason for hiding this comment

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

if IconInfo or BuffInfo aren't actually disposable types then there isn't benefit to implementing IDisposable here. Counterintuitively, setting collections to null can sometimes delay their GC cleanup due to funky .NET things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have 0 idea how either works tbh, if they're made to be disposable, then disposable they are

{
public readonly List<IconInfo> icons;
public readonly List<BuffInfo> buffs;
public List<IconInfo> icons { get; private set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should use init instead of private set if these are still just being initialized in the constructor.

}
}

// source: trust me bro
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you provide a source for this? Not that I don't trust you, but we aren't actually running a game so using the slower, but more accurate, power methods is likely preferable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The accuracy difference is about 1 in 4 thousand decimal places, so very very tiny, and it's an almost 20x speed up, this is my source: https://martin.ankerl.com/2007/10/04/optimized-pow-approximation-for-java-and-c-c/

Copy link
Collaborator

Choose a reason for hiding this comment

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

All good, just wondering what dark magic you were incanting.

Copy link
Collaborator

@horfius horfius 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 much cleaner than before. Only one actual error I can see although some of the other cleanup would be nice, but optional.

loadingTitleFmg.Entries.Clear();
loadingTextFmg.Entries.Clear();

int id = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You forgot to increment this per loop.

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.

3 participants