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

Housekeeping - uneccessary code #787

Open
white-haired-uncle opened this issue May 8, 2024 · 10 comments
Open

Housekeeping - uneccessary code #787

white-haired-uncle opened this issue May 8, 2024 · 10 comments

Comments

@white-haired-uncle
Copy link
Collaborator

I've been wondering just how much clutter is being carried around, stored in every save. While I doubt it would make a noticable difference to anyone in terms of performance, memory, or disk space, I'm still curious. This is a thread to consider related issues. We got rid of item_list (in WML), what else is there?

The help menu is rarely used. And it looks like its entire contents are stored in every save. Should be easy enough to reconfigure that so that it is only loaded when accessed.

I'm looking at some saves from part1 which include a decent amount of redeem code. If nothing else, this could be broken out so that it is only loaded in part2, and ideally only loaded once you achieve redeem (at which point, the soul eater code becomes stale -- I think).

I doubt any cleanup would provide much benefit, and trying to get to granular would probably just result in added complexity, but it's probably worth looking at just the low hanging fruit.

@Dugy
Copy link
Owner

Dugy commented May 8, 2024

Smaller save files are more convenient to work with overall because text editors are a little slow from editing save files that are several megabytes large.

And yes, there is a lot of unnecessary stuff. There used to be much more of it, but now, a lot of stuff was moved to lua, so if I do random sampling of a save file's contents, it seems to be mostly campaign statistics, units mostly on the recall list, and scenario replay data. The units themselves take most space by far.

So if you want to move the help menu into a helper unit type or move the redeem code into lua, you can, but I don't think it's worth the effort.

The best way to reduce the save file size right now is to shorten the code of units. Removing all the properties of items except for number and slot is a low hanging fruit.

@white-haired-uncle
Copy link
Collaborator Author

The best way to reduce the save file size right now is to shorten the code of units. Removing all the properties of items except for number and slot is a low hanging fruit.

How much can be removed? Stuff like gladiator_drop= and sort=. Descriptions for anything that's not a weapon would probably be the big one. But isn't most of it necessary for the object to function properly?

@white-haired-uncle
Copy link
Collaborator Author

white-haired-uncle commented May 8, 2024

These are things from global_events which look like they could at least be only loaded in one part, not both:

    {REDEEMING}
    {LOTI_DEMON_HACK}   # Nope, needed for BZB
    {LOTI_MAKE_BLACK_SOUL}
    {LOTI_MAKE_DOPPELGANGER}
    {LOTI_KILL_ALL_DOPPELGANGERS}

@Dugy
Copy link
Owner

Dugy commented May 8, 2024

I think its effect would be neglectful. Also, all of those except for redeeming are events used as functions, which should have been lua-based WML tags instead.

@white-haired-uncle
Copy link
Collaborator Author

#ifdef CAMPAIGN_LEGEND_OF_THE_INVINCIBLES_PART_I
    {REDEEMING}
#endif

saves all of about 3K (30K uncompressed), which is probably less than 1% of most part1 saves. Almost nothing for the rest.

I was thinking you could remove a lot of the cruft from items on units on recall, but that would mess with unit_preview_pane, require messing with Items->Items on units on recall, and extra code to "collapse/expand" items as units move to/from recall.

Doesn't seem like there's much to do here.

@Dugy
Copy link
Owner

Dugy commented May 11, 2024

I was thinking you could remove a lot of the cruft from items on units on recall, but that would mess with unit_preview_pane, require messing with Items->Items on units on recall, and extra code to "collapse/expand" items as units move to/from recall.

Yes, doing this would probably remove a double digit percentage of size, but right now, the codebase probably isn't ready for it.

@white-haired-uncle
Copy link
Collaborator Author

white-haired-uncle commented May 24, 2024

There's a decent amount of animation code in save files (every unit with iceform, etc). I actually went looking for animation code because I noticed some unit types have animation defined as foo-1, foo-2, foo-3...foo-12 while other use foo[1~12]. For units like "demons" in Jungle Hell, this could add up to a lot of lines of code. However, it looks like unit_type animation definitions are not stored in the save. Makes me wonder if animations within [effect] could be loaded dynamically? No idea how, just saving the thought.

Also noticed several hundred lines of BZB code in a scenario with no BZB. This should be easy enough to get rid of, don't create the events unless saved_turns>=500 (better yet, don't load them until they're needed). Nope, it already does something like that, but the prestart event that (doesn't) load the events is itself stored (in the save for turn 24), so there's a copy of the event code anyway.

Hmm, don't know why there was so much BZB code in the save, need to look again. What there is in utils.cfg has a lot of macros, so it's probably not worth the effort to load dynamically vs just re-writing in lua. Then again, the post-processed code is right there in the save file, not much work needed.

@Dugy
Copy link
Owner

Dugy commented May 24, 2024

There's a decent amount of animation code in save files (every unit with iceform, etc). I actually went looking for animation code because I noticed some unit types have animation defined as foo-1, foo-2, foo-3...foo-12 while other use foo[1~12].

Yes, that part could be improved. The [1~12] format was added after much of the animation code was written.

Makes me wonder if animations within [effect] could be loaded dynamically? No idea how, just saving the thought.

No, at least not without moving all legacy-related animations into unit types. If the animation doesn't come from the unit type, it has to be in an [effect].

Also noticed several hundred lines of BZB code in a scenario with no BZB. This should be easy enough to get rid of, don't create the events unless saved_turns>=500 (better yet, don't load them until they're needed).

Yes, they could be loaded when needed from some resource file, like you've done before.

Nope, it already does something like that, but the prestart event that (doesn't) load the events is itself stored (in the save for turn 24), so there's a copy of the event code anyway.

I didn't do that for optimisation, it was just somewhat convenient for implementation.

@white-haired-uncle
Copy link
Collaborator Author

The best way to reduce the save file size right now is to shorten the code of units. Removing all the properties of items except for number and slot is a low hanging fruit.

How much can be removed? Stuff like gladiator_drop= and sort=. Descriptions for anything that's not a weapon would probably be the big one. But isn't most of it necessary for the object to function properly?

Looking at this again, probably not.

Currently looking at loti.unit.add_item. I add the item there with just name/number/sort, but it's picking up the description/etc from somewhere (update_stats?).

@Dugy
Copy link
Owner

Dugy commented May 25, 2024

Currently looking at loti.unit.add_item. I add the item there with just name/number/sort, but it's picking up the description/etc from somewhere (update_stats?).

I don't know for sure. update_stats definitely reviews all items the unit has, so it's the most likely culprit.

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

No branches or pull requests

2 participants