Skip to content

Implement Fast Level Loading#1072

Open
DashingCat wants to merge 3 commits into
EverestAPI:devfrom
DashingCat:fast-level-loading
Open

Implement Fast Level Loading#1072
DashingCat wants to merge 3 commits into
EverestAPI:devfrom
DashingCat:fast-level-loading

Conversation

@DashingCat

Copy link
Copy Markdown
Contributor

This PR implements "Fast Level Loading" (like Fast Texture Loading, but for levels).

In collabs, levels can take a lot of time to load (especially since texture loading times have already been improved).

The implementation uses Parallel.For (which uses CPU core count as degree of parallelism), is enabled by default (this could be changed), and logs level loading order (to help diagnose potential issues).

It adds a ThreadStatic attribute to stringLookup in BinaryPacker, so each thread has its own lookup.

It also adds a lock in MapData, because map data processors from mods don't expect concurrent access and may fail.

During testing, these changes improved loading times on my end, loading "Strawberry Jam Collab" levels takes about 6 seconds before changes, and about 4 seconds after changes.

@maddie480-bot maddie480-bot added the 0: draft This PR is not ready for review yet (bot-managed) label Jan 14, 2026
@DashingCat DashingCat marked this pull request as ready for review January 15, 2026 21:25
@JaThePlayer

JaThePlayer commented Jan 15, 2026

Copy link
Copy Markdown
Member

Very nice! Would probably be a good idea to add some virtual bool IsThreadSafe => false; to MapDataProcessors, so that mods can work towards thread safety for their processors. If all registered processors are declared thread-safe, then the lock could be omitted and we'd probably see a larger perf boost. (And perhaps add some logging for non-thread-safe processors so we know what's blocking us?)

@maddie480-bot maddie480-bot added 1: review needed This PR needs 2 approvals to be merged (bot-managed) and removed 0: draft This PR is not ready for review yet (bot-managed) labels Jan 15, 2026
@DashingCat

Copy link
Copy Markdown
Contributor Author

Very nice! Would probably be a good idea to add some virtual bool IsThreadSafe => false; to MapDataProcessors, so that mods can work towards thread safety for their processors. If all registered processors are declared thread-safe, then the lock could be omitted and we'd probably see a larger perf boost. (And perhaps add some logging for non-thread-safe processors so we know what's blocking us?)

Thanks!

From my testing (on the "Strawberry Jam Collab"), the locking doesn't have much impact on the performance boost (less than half a second).

I'm not sure getting mods towards thread safety in their map data processors is needed at this point, because it could lead to issues if mods are declared thread-safe but aren't in practice; and thread synchronization issues can be hard to diagnose and fix.

maddie480
maddie480 previously approved these changes Mar 1, 2026

@Wartori54 Wartori54 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good other than my comment, will approve once it gets resolved.

Comment thread Celeste.Mod.mm/Patches/AreaData.cs Outdated
}
}

if (CoreModule.Settings.FastLevelLoading ?? true) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps we should ship it as disabled by default, advertise it once it lands on stable, and schedule a PR to make it enabled by default once some time has passed. In order to prevent what already happened with the fast FMOD loading.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @Wartori54 for the feedback, the latest commit adds a toggle for the feature in the "Mod Options", which is disabled by default.

A future PR could then set the field FastLevelLoading to true.

@DashingCat DashingCat force-pushed the fast-level-loading branch from e784caa to 7f4fdd8 Compare June 25, 2026 09:43
@DashingCat DashingCat requested a review from Wartori54 June 25, 2026 09:49
[SettingInGame(false)]
[SettingIgnore] // TODO: Show as advanced setting.
public bool? FastLevelLoading { get; set; } = null;
public bool FastLevelLoading { get; set; } = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's better to keep this as = null, so that migration is easier. This allows us to freely change the meaning of the null value without having to modify the users' settings file.
I can't quite remember how is bool? handled in the in game ui, some extra work may be needed ui wise to get this to work.

@DashingCat DashingCat Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, these comments were addressed in the latest commit; bool? are not handled in the "Mod Settings" UI but a custom handler was made.

image

@DashingCat DashingCat requested a review from Wartori54 June 25, 2026 14:15
@Wartori54 Wartori54 dismissed maddie480’s stale review June 25, 2026 15:29

Significant changes in PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1: review needed This PR needs 2 approvals to be merged (bot-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants