Skip to content

Conversation

@minetoblend
Copy link
Contributor

@minetoblend minetoblend commented Sep 1, 2025

Closes #34846

Turns out the editor player was relying on the 1-frame delay before hitobject drawables would become alive prior to #34830.
Issue here was that with hitobjects becoming alive immediately, they already had a chance to update their state to "miss" before the player can manually set their result now.
This lead to a null pointer exception in the playfield's update look due to JudgementResult.RawTime being null.

Assigning a time to the result did fix the crash but the logic of marking the objectse was still broken, so tried to hook things up to update before the hitobject has a chance to update it's own state. Not 100% confident in this fix quality wise though so maybe reverting #34830 is a better option.

@minetoblend minetoblend changed the title Fix crash when marking previous objects as hit Fix crash when trying to test map in the editor Sep 1, 2025
@bdach
Copy link
Collaborator

bdach commented Sep 1, 2025

very curious to hear why this is being pr'd 30 minutes after i self assigned the issue for investigation

they already had a chance to update their state to "miss" before the player can manually set their result now

please point out the exact mechanism of this failure, this text description is insufficient for my understanding of the issue

@minetoblend
Copy link
Contributor Author

very curious to hear why this is being pr'd 30 minutes after i self assigned the issue for investigation

Already answered on discord gonna skip over over that

please point out the exact mechanism of this failure, this text description is insufficient for my understanding of the issue

there's 2 failure mechanisms here

  • due to the hitobjects having undergone an update-loop already, if they update their result on the first update loop their entry lands in Playfield.judgedEntries. The editor player would then replace the entry's result with a new one, which was missing a value for Result.RawTime, leading to an assertion error (or null pointer exception in production) in
    Debug.Assert(result?.RawTime != null);
    if (Time.Current >= result.RawTime.Value)
  • assigning a value to it does prevent the crash, but since the object got into miss state through the default flow it will already have notified all sorts of other playfield components, leading to the miss sound being played and the judgment drawables being shown on the playfield.

@minetoblend
Copy link
Contributor Author

minetoblend commented Sep 1, 2025

I'm very puzzled by the test failure here. I looked at it with a debugger and it looks like Player.load() is never being called, leading to DrawableRuleset never being set? I double checked the order in which things are supposed in osu-framework and this should definitely have run before LoadAsyncComplete.
nvm just saw that it won't initialize when playableBeatmap is null

@minetoblend minetoblend force-pushed the fix/editor-player-crash branch from 972be5e to 689cc27 Compare September 1, 2025 09:12
Marvin Schürz added 2 commits September 1, 2025 12:10
Running that there caused a test failure due to modifying drawables' transforms outside the update thread
@minetoblend
Copy link
Contributor Author

minetoblend commented Sep 1, 2025

There was one more test failure due to a mismatch in which objects had a result applied and which ones were received by the score processor, which could lead to the score processor never receiving the results for some hitobjects and it's HasCompleted value never becoming true.
It does pass now but I'm not 100% confident that there's no way this can't still somehow fail now.

Marvin Schürz and others added 3 commits September 1, 2025 13:13
`LastOrDefault()` is arbitrary, and I hope this doesn't matter for
anything, because if it does, then that's utterly *horrifying*.
@bdach bdach added area:editor type/reliability Deals with game crashing or breaking in a serious way. labels Sep 2, 2025
bdach
bdach previously approved these changes Sep 2, 2025
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

horrible hack but the previous code was already a hack so I'm not going to split hairs

@bdach bdach requested review from peppy and smoogipoo September 2, 2025 08:59
Copy link
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Just a minor one.

@bdach bdach force-pushed the fix/editor-player-crash branch from fd193c4 to 4ed72ef Compare September 2, 2025 10:42
@peppy peppy merged commit f414dd7 into ppy:master Sep 2, 2025
6 of 9 checks passed
@minetoblend minetoblend deleted the fix/editor-player-crash branch September 2, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:editor size/M type/reliability Deals with game crashing or breaking in a serious way.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tachyon hangs and then crashes when trying to testplay a beatmap in the editor

3 participants