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

Fix overlapping chat links crashing the game #25627

Merged
merged 7 commits into from Dec 2, 2023

Conversation

Susko3
Copy link
Member

@Susko3 Susko3 commented Dec 1, 2023

Depends on:

Partial fix for #25614. Partial because it doesn't produce the ideal parsing results. Ideal would be: "osu://edit/00:12:345 (1,2) - Test?"
If you think reaching the ideal is not required, the issue could be marked as completed.

image


Fixes cases where the same string could be parsed into multiple overlapping links. This was caused by the faulty "Check for encapsulated links" logic.

This PR:

  1. hardens LinkFlowContainer so it doesn't crash if overlapping links are provided (it'll ignore any overlapping links and log the problem)
  2. changes the "encapsulated links" logic to be more general and handle all cases of overlapping links
    • I didn't verify/think about the maths behind the "encapsulated links" logic, but it clearly produces wrong results
    • overlapping links are already checked and accounted for in handleAdvanced(), so I took the new logic from there

They throws `ArgumentOutOfRangeException` on the first drawable Update()
This actually fixes the problem and makes the tests pass
@pull-request-size pull-request-size bot added size/S and removed size/L labels Dec 1, 2023
osu.Game/Online/Chat/MessageFormatter.cs Outdated Show resolved Hide resolved
@bdach bdach merged commit ee2a06b into ppy:master Dec 2, 2023
15 of 17 checks passed
@Susko3 Susko3 deleted the fix-overlapping-chat-links branch December 2, 2023 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants