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: Don't save industry history if cargo slot isn't used. #11133

Merged
merged 1 commit into from Jul 14, 2023

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented Jul 13, 2023

Motivation / Problem

Industries have 16 slots which may produce cargo, and we save two months of production/transported history for each slot for each industry.

In many cases (dependent on NewGRFs), 14 of the 16 slots won't actually ever be used as cargo slot is CT_INVALID.

Description

Skip saving history if the slot is CT_INVALID by setting the saved length to 0.

This avoids saving history of 16 slots per industry when in many cases (NewGRF dependent) only a couple are used.

Does not need a saveload bump as the loading code is already told how much data there is.

Limitations

As savegames are compressed, this doesn't particularly save much space... on a 4096x4096 map with high industries the resultant file size difference is negli:ble.

Vanilla: 24,766,936 bytes
Patched: 24,760,648 bytes

This may be of more benefit with #10541, but... no idea.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@PeterN
Copy link
Member Author

PeterN commented Jul 13, 2023

Oops, this should use IsValidCargoId() :-)

This avoids saving history of 16 slots per industry when in many cases (NewGRF dependent) only a couple are used.
@PeterN PeterN merged commit c3d1264 into OpenTTD:master Jul 14, 2023
19 checks passed
@PeterN PeterN deleted the industry-history-sl branch July 14, 2023 10:12
@ldpl
Copy link
Contributor

ldpl commented Jul 14, 2023

Did a bit of testing, seems to be a slight improvement overall. Though uncompressed size doesn't add up, I'd expect about 32 * 2 * 14 = 896 bytes per industry reduction based on the description of this PR, but got 119 instead.
Screenshot from 2023-07-14 15-00-21

Timing patch:
savetiming.txt
Command: ./openttd -G 1 -g -v null:ticks=55000 && ls -lsa ~/.openttd/save/autosave/exit.sav

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

3 participants