Add debug map option to critical error handler by balt-dev · Pull Request #1121 · EverestAPI/Everest

balt-dev · 2026-06-01T18:07:36Z

Adds an option to enter the debug level editor to the critical error handler.

microlith57

lgtm, thanks for the contribution!

Kalobi · 2026-06-18T17:46:56Z

I'm not a fan of having an option cut off at the bottom and having stuff move around for no reason while navigating the menu when there's plenty of space on screen to hold all of the options.
Also, this is not within the scope of this PR since you were just following the structure of the existing code, but I think it might be a good idea to make these texts localizable.

SnipUndercover · 2026-06-18T18:03:55Z

Also, this is not within the scope of this PR since you were just following the structure of the existing code, but I think it might be a good idea to make these texts localizable.

We have a drafted #991 that does that.

balt-dev · 2026-06-19T23:44:25Z

I'm not a fan of having an option cut off at the bottom and having stuff move around for no reason while navigating the menu when there's plenty of space on screen to hold all of the options. Also, this is not within the scope of this PR since you were just following the structure of the existing code, but I think it might be a good idea to make these texts localizable.

The localization isn't my wheelhouse, and I didn't want to mess with the size of the player drawing in case someone else would take issue with that. It's also future-proofing for if more entries are added to the handler.

balt-dev · 2026-06-21T04:13:05Z

Went and redid a lot of the UI code as per discussion with Kalobi on Discord.

balt-dev · 2026-07-01T19:36:56Z

If I may request, could this be squash-merged if/when it's approved? It should make things cleaner.

Wartori54

Good first PR! Just needs a bit more polish and taking a step back and thinking what is truly needed in here.

Wartori54 · 2026-08-17T13:04:33Z

+            DisplayState state = displayState switch {
+                "overlay" => DisplayState.Overlay,
+                "cleanscene" => DisplayState.CleanScene,
+                _ => DisplayState.Initial,
+            };


Apart from having missed the DisplayState.BlueScreen in the switch, this is a bit misleading.
Calling HandleCriticalError with a given DisplayState will advance to the next possible display state, meaning criterror overlay will use a clean scene instead of being an overlay.
I don't think we should add the ability to force a DisplayState but this method should call HandleCriticalError with the greatest DisplayState that comes before the specified displayState, in my opinion.

Wartori54 · 2026-08-17T13:29:14Z


+        // Forcibly saves the game.
+        private void ManuallySaveGame() {
+            if (SaveData.Instance is null) return;


I'd rather throw here rather than return, all callers already check for this.
This could be static too.

Wartori54 · 2026-08-17T14:03:18Z

+            // First, we find how much space we have worst-case
+            optMenu.ItemSpacing = 0;
            optMenu.RecalculateSize();
-
-            if (UsePlayerSprite) {
-                optMenu.Position = new Vector2(Celeste.TargetWidth * 0.15f, Celeste.TargetHeight * 0.55f);
-                optMenu.Justify = new Vector2(0.5f, 0f);
-
-                // Reduce item spacing if there are too many items
-                if (optMenu.Position.Y + optMenu.Height > Celeste.TargetHeight * 0.85f) {
-                    optMenu.ItemSpacing = 0;
-                    optMenu.RecalculateSize();
+            PlayerSpriteSize = MaxPlayerSpriteSize;
+
+            float originalSpriteSize = Celeste.TargetHeight * PlayerSpriteSize;
+
+            // Check how much space we have left over
+            float verticalDeadSpace = MaxOptionsHeight - optMenu.Height;
+            if (UsePlayerSprite) verticalDeadSpace -= originalSpriteSize;
+            if (verticalDeadSpace < 0 && UsePlayerSprite) {
+                // We need to shrink the player sprite to fit the screen
+                PlayerSpriteSize = Math.Min((originalSpriteSize + verticalDeadSpace) / Celeste.TargetHeight, MaxPlayerSpriteSize);
+                if (PlayerSpriteSize <= 0) {
+                    Logger.Warn("crit-error-handler", "Not enough vertical space for player sprite, disabling");
+                    disablePlayerSprite = true;
+                    PlayerSpriteSize = 0;
                }
-            } else {
-                optMenu.Position = new Vector2(Celeste.TargetWidth * 0.15f, Celeste.TargetHeight * 0.6f);
-                optMenu.Justify = new Vector2(0.5f, 0.5f);
+
+                verticalDeadSpace += originalSpriteSize - PlayerSpriteSize * Celeste.TargetHeight;
            }
+
+            int optionCount = ((patch_TextMenu)optMenu).Items.Count;
+
+            float optionSpacing = verticalDeadSpace / optionCount;
+
+            if (optionSpacing < 0)
+                // We are completely out of room. RIP. We intentionally don't clamp optionSpacing to >=0 to keep everything on screen even with overlap
+                Logger.Warn("crit-error-handler", $"Not enough vertical space for options menu, forcing overlap");
+
+            optMenu.ItemSpacing = Math.Min(optionSpacing, MaxOptionSpacing);
+            optMenu.RecalculateSize();
+
+            if (optMenu.Height - 1f > MaxOptionsHeight) // -1f to give a leeway for float shenanigans
+                Logger.Warn("crit-error-handler", $"Got incorrect height for option menu, is this a bug? (Expected {MaxOptionsHeight}, got {optMenu.Height})");
+
+            // We center the options within the available space ONLY if the player sprite isn't there,
+            // since it looks fine uncentered with the player but awful without
+            float availableSpace = MaxOptionsHeight - optMenu.Height;
+
+            float optY = (Celeste.TargetHeight - MaxOptionsHeight) / 2f;
+            if (UsePlayerSprite)
+                optY += Celeste.TargetHeight * PlayerSpriteSize;
+            else
+                optY += availableSpace / 2;
+
+            optMenu.Position = new Vector2(Celeste.TargetWidth * 0.15f, optY + optMenu.Height / 2);
+            optMenu.Justify = new Vector2(0.5f, 0.5f);


Not sure about this, feels like an overly complicated way to auto layout something that will only have two distributions: overworld menu, and in game menu (+ player or no player sprite).
Wouldn't something like the old code with adjusted constants work? Or just a fixed y position for the start of the buttons, and then "smartly" nudge them up if the list would go out of frame?
This menu will have to get rewritten entirely with submenus if we keep adding more buttons anyway.

I'm really torn on this. On the one hand, I physically can't fit the debug editor option into the old layout without changing the code somewhat, and so if I'm going to be changing the code at all, I'd rather do it well than half-ass it. On the other, though, I'd honestly rather go back to the old source code and shrink the scope of this PR, if at least to get the main feature I want (the debug editor option) merged, with the layout being tackled later on - I'm not the most comfortable working with Everest internals, and given I'm not a maintainer, I feel like anything greater than this in scope, and even what I have now, should fall under, while deserved, very harsh criticism, since ANY change to Everest's internal systems has a very far-reaching impact. I've been laughed out of a project before due to terrible code quality, and I don't want to repeat that.

If you really want me to, I can completely rewrite the menu to have submenus and such, but as it stands, that feels like the only option apart from rejecting this PR entirely. As I said, I'd rather do it well than half-ass it.

Not sure about this, feels like an overly complicated way to auto layout something that will only have two distributions: overworld menu, and in game menu (+ player or no player sprite). Wouldn't something like the old code with adjusted constants work? Or just a fixed y position for the start of the buttons, and then "smartly" nudge them up if the list would go out of frame? This menu will have to get rewritten entirely with submenus if we keep adding more buttons anyway.

@Wartori54 ...thinking about it more, I feel I might've gotten the wrong message from this. Could you elaborate on what you want me to do here?

balt-dev · 2026-08-28T19:13:24Z

!? I didn't close the PR...
Github?

balt-dev · 2026-08-28T19:13:48Z

Here.

balt-dev · 2026-08-28T19:14:01Z

Okay... one moment, then.

balt-dev · 2026-08-28T20:02:15Z

I've decided that doing anything than just simply adding the debug map option is probably overscoping for this PR, given my lack of experience with Everest's codebase.

balt-dev · 2026-08-28T21:47:40Z

Ambiguous match found for 'Celeste.Mod.UI.CriticalErrorHandler System.Runtime.ExceptionServices.ExceptionDispatchInfo HandleCriticalError(System.Runtime.ExceptionServices.ExceptionDispatchInfo)'.

why would you compile my code then

balt-dev · 2026-08-29T17:34:15Z

(08/29/2026 17:33:26) [Everest] [Error] [CelesteTAS] Failed to get method 'HandleCriticalError' on type 'Celeste.Mod.UI.CriticalErrorHandler'
--------------------------------
System.Reflection.AmbiguousMatchException: Ambiguous match found for 'Celeste.Mod.UI.CriticalErrorHandler System.Runtime.ExceptionServices.ExceptionDispatchInfo HandleCriticalError(System.Runtime.ExceptionServices.ExceptionDispatchInfo)'.

Are you kidding me?

Wartori54

LGTM, just two non-blocking comments.

Wartori54 · 2026-08-29T19:08:23Z

+            if (CurrentHandler == this) return;
+            RemoveSelf(); // The rest of the coroutine is never run


What's the purpose of these two lines?

Doing what happens after Fade immediately, so that I don't have to hack in a NoFade flag.

Those two lines ensure there are no duplicate crash handlers, they are run at the end of the Routine as well, but don't affect the fade.

psyGamer · 2026-08-29T19:56:42Z

-        public static ExceptionDispatchInfo HandleCriticalError(ExceptionDispatchInfo error) {
+        // Done so that older mods that use this don't crash due to a linking error
+        public static ExceptionDispatchInfo HandleCriticalError(ExceptionDispatchInfo error) => HandleCriticalError(error, DisplayState.Initial);
+
+        public static ExceptionDispatchInfo HandleCriticalError(ExceptionDispatchInfo error, DisplayState defaultDisplayState) {


I've checked and the only mod which uses this method directly and would break is FFFFF.
With the change however, it breaks CelesteTAS due to an ambigous match.

I am of course a bit biased, but imo it would make more sense to remove the backwards compat, since:
a) more people are using CelesteTAS and even if an update exists, people are known to not update their mods
b) It reduces tech debt
c) The developer of FFFFF has been made aware that internal Everest changes will not take the resonsbility for keeping the map working

The counter argument to this is that HandleCriticalError is a public API and we should keep those working.
Also in CelesteTAS you could push out an update now that filters for the overload with the most amount of arguments, and that would work for current Everest and for future Everest once this change lands.
Adding a new overload when you need to introduce a new parameter on a public API is something that the BCL does extensively and I'd say its a good practice even.

I didn't really consider it a public API. I suppose it's public which is all the qualification it needs?

maddie480-bot added the 1: review needed This PR needs 2 approvals to be merged (bot-managed) label Jun 1, 2026

microlith57 approved these changes Jun 2, 2026

View reviewed changes

Comment thread Celeste.Mod.mm/Mod/UI/CriticalErrorHandler.cs Outdated

balt-dev changed the title ~~Add debug editor option and scrolling to critical error handler~~ Add debug editor option and improve layout handling of critical error handler Jun 21, 2026

microlith57 added the 4: should be squashed PR with a messy commit history that should be squash merged label Jul 1, 2026

balt-dev force-pushed the feature-criterrorhandler-debugeditor branch from 5d73166 to 231a3f3 Compare July 19, 2026 01:25

Wartori54 requested changes Aug 17, 2026

View reviewed changes

maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Aug 17, 2026

balt-dev force-pushed the feature-criterrorhandler-debugeditor branch from 231a3f3 to fca56d6 Compare August 28, 2026 03:52

balt-dev closed this Aug 28, 2026

balt-dev force-pushed the feature-criterrorhandler-debugeditor branch from fca56d6 to 33d811a Compare August 28, 2026 19:12

Initial work on critial error handler stuff

4e99850

balt-dev reopened this Aug 28, 2026

balt-dev marked this pull request as draft August 28, 2026 19:16

Add debug map to options menu

03c2ac0

balt-dev changed the title ~~Add debug editor option and improve layout handling of critical error handler~~ Add debug map option to critical error handler Aug 28, 2026

maddie480-bot added 0: draft This PR is not ready for review yet (bot-managed) and removed 2: changes requested This PR cannot be merged because changes were requested (bot-managed) labels Aug 28, 2026

balt-dev added 2 commits August 28, 2026 15:20

Add criterror command

f3eb2d3

Readd keyboard shortcuts

c403be1

balt-dev requested a review from microlith57 August 28, 2026 20:38

balt-dev requested a review from Wartori54 August 28, 2026 20:38

balt-dev marked this pull request as ready for review August 28, 2026 20:38

maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 0: draft This PR is not ready for review yet (bot-managed) labels Aug 28, 2026

balt-dev added 2 commits August 28, 2026 17:29

Fix TAS tools exploding because of insane hook linker shenanigans

c194d38

Nullability is weird in C#

150b26b

Wartori54 reviewed Aug 29, 2026

View reviewed changes

Update CriticalErrorHandler.cs

fc876bc

This comment was marked as duplicate.

Sign in to view

psyGamer reviewed Aug 29, 2026

View reviewed changes

		if (CurrentHandler == this) return;
		RemoveSelf(); // The rest of the coroutine is never run

Uh oh!

Conversation

balt-dev commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

microlith57 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kalobi commented Jun 18, 2026

Uh oh!

SnipUndercover commented Jun 18, 2026

Uh oh!

balt-dev commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balt-dev commented Jun 21, 2026

Uh oh!

balt-dev commented Jul 1, 2026

Uh oh!

Wartori54 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balt-dev Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balt-dev commented Aug 28, 2026

Uh oh!

balt-dev commented Aug 28, 2026

Uh oh!

balt-dev commented Aug 28, 2026

Uh oh!

balt-dev commented Aug 28, 2026

Uh oh!

balt-dev commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balt-dev commented Aug 29, 2026

Uh oh!

Wartori54 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

balt-dev commented Jun 1, 2026 •

edited

Loading

balt-dev commented Jun 19, 2026 •

edited

Loading

balt-dev Aug 28, 2026 •

edited

Loading

balt-dev commented Aug 28, 2026 •

edited

Loading