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

Improve loading strings from EDT files #1988

Merged
merged 2 commits into from
May 15, 2024

Conversation

momoko-h
Copy link
Contributor

@momoko-h momoko-h commented May 7, 2024

This commit adds the following:

  1. A new interface IEDT. This very simple interface has just one method to get a single string from an EDT file.
  2. Class ClassicEDT is a helper class for files in the original .edt format. It does not do much, it simply opens the file and takes care of offset calculations.
  3. Class JsonEDT is only available for mods via the ModPackContentManager. It uses a classic .edt file as a fallback option and allows to override individual strings with entries taken from a .json file.
  4. Class EDTFile is the intended replacement for the current loadEncryptedString calls. It contains the details needed to interpret the layout of the supported .edt files.

As Derek mentioned, the quote files in the mercedt folder were already done and selaux added support for itemdesc.edt in #1575. This PR adds aimbios.edt, mercbios.edt and help.edt and should be flexible enough for the remaining files in binarydata.

This seems to work but I'm marking it as a draft because I'd like to get some feedback about the general design and more testing first. Don't forget to enable the test-json-dialogs mod to see some changes.

This commit adds the following:
1. A new interface IEDT. This very simple interface has
just one method to get a single string from an EDT file.
2. Class ClassicEDT is a helper class for files in the
original .edt format. It does not do much, it simply
opens the file and takes care of offset calculations.
3. Class JsonEDT is only available for mods via the
ModPackContentManager. It uses a classic .edt file as a
fallback option and allows to override individual strings
with entries taken from a .json file.
4. Class EDTFile is the intended replacement for the
current loadEncryptedString calls. It contains the details
needed to interpret the layout of the supported .edt files.
@momoko-h
Copy link
Contributor Author

momoko-h commented May 7, 2024

Here is a screenshot to demonstrate that this addresses the problem mentioned in #1986. It's possible to fill the entire available area with text now.

Screenshot_20240507_083338

@lynxlynxlynx lynxlynxlynx added the externalization Extracting hard-coded data to externalized files label May 7, 2024
@lynxlynxlynx
Copy link
Member

Nice work! I only noticed a blog/blob typo, but have no comments on the architecture.

Is there any reason we wouldn't use the same implementation for the existing users, as a second step to this?

@momoko-h
Copy link
Contributor Author

momoko-h commented May 8, 2024

VanillaItems also uses a schema so that would require a bit more work and that's time that is probably better spent adding more .edt files.

The DefaultContentManager version of loadDialogQuoteFromFile could easily be converted but the ModPack version expects the .edt.json file to contain a complete set of all quotes in that file so that it can cache these strings so that would require a redesign that probably wouldn't have much benefit.

Also changes this method to no longer return a pointer to
a string which simplifies its only user, GetDialogue().
@momoko-h momoko-h marked this pull request as ready for review May 9, 2024 07:25
@lynxlynxlynx lynxlynxlynx merged commit aabd7df into ja2-stracciatella:master May 15, 2024
7 of 8 checks passed
@lynxlynxlynx
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
externalization Extracting hard-coded data to externalized files modding proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increas the displayed character limit, so that the full lines in AIM and MERC Bios is useable.
2 participants