Skip to content

Refactor section creation errors, and simplify output for single errors#1964

Merged
Rangi42 merged 3 commits into
gbdev:masterfrom
Rangi42:sect-errors
May 21, 2026
Merged

Refactor section creation errors, and simplify output for single errors#1964
Rangi42 merged 3 commits into
gbdev:masterfrom
Rangi42:sect-errors

Conversation

@Rangi42

@Rangi42 Rangi42 commented May 1, 2026

Copy link
Copy Markdown
Contributor

The refactoring here is because the #define sectError macro was an awkward hack, relying on each of its callers to have a variable specifically called nbSectErrors.

The simplification is because most sectErrors are exclusive (note all the if-else chains) so you rarely end up with more than one (note that I had to add a new test for multiple errors with the same section). Comparing these two outputs:

  • A non-fatal error that says "Section had this specific flaw", followed by FATAL "Cannot create section "<name>" (1 error)"
  • A FATAL "Section "<name>" had this specific flaw"

I find the latter new output easier to read, and less likely for inexperienced users to tunnel-vision read the very last error and miss its context. (Especially when they just post a screenshot/quote of the last error.)

@Rangi42 Rangi42 added this to the 1.0.2 milestone May 1, 2026
@Rangi42
Rangi42 requested a review from ISSOtm May 1, 2026 11:17
@Rangi42 Rangi42 added enhancement Typically new features; lesser priority than bugs rgbasm This affects RGBASM refactoring This PR is intended to clean up code more than change functionality labels May 1, 2026

@ISSOtm ISSOtm 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.

There's a reason I'm not writing C++ anymore...

Comment thread include/asm/warning.hpp
Comment thread src/asm/section.cpp
Comment thread src/asm/section.cpp Outdated
Comment thread src/asm/warning.cpp
@Rangi42
Rangi42 requested a review from ISSOtm May 11, 2026 13:07

@ISSOtm ISSOtm 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.

Well, I still don't like these changes too much, but we've established that this is due to value differences that the solution to is me switching to Rust. So I'm not going to block this any longer.

@Rangi42
Rangi42 merged commit d56dbbb into gbdev:master May 21, 2026
50 of 51 checks passed
@Rangi42
Rangi42 deleted the sect-errors branch May 21, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Typically new features; lesser priority than bugs refactoring This PR is intended to clean up code more than change functionality rgbasm This affects RGBASM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants