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

Convert many Topic overrides to use OnTopic instead #4746

Merged
merged 17 commits into from
Feb 1, 2025

Conversation

out-of-phaze
Copy link
Member

Description of changes

HEAVY WIP. Will probably need testing but that'll be a huge pain with how much this touches, so it might need to be broken up.

Why and what will this PR improve

Overriding Topic instead of OnTopic causes issues with missing parent call functionality and TOPIC_BLAH return values not being respected. This is better.

@out-of-phaze out-of-phaze added the work in progress This PR is under development and shouldn't be merged. label Jan 16, 2025
@out-of-phaze out-of-phaze added ready for review This PR is ready for review and merge. and removed work in progress This PR is under development and shouldn't be merged. labels Jan 29, 2025
@out-of-phaze out-of-phaze marked this pull request as ready for review January 29, 2025 20:35
MistakeNot4892
MistakeNot4892 previously approved these changes Jan 31, 2025
Copy link
Contributor

@MistakeNot4892 MistakeNot4892 left a comment

Choose a reason for hiding this comment

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

Not going to pretend I read the diff in detail, but nothing leaped out as obviously wrong.

@MistakeNot4892 MistakeNot4892 added the has conflicts This PR needs updating and conflict resolution before it can be merged. label Feb 1, 2025
@MistakeNot4892 MistakeNot4892 removed the has conflicts This PR needs updating and conflict resolution before it can be merged. label Feb 1, 2025
@MistakeNot4892 MistakeNot4892 merged commit 602924b into NebulaSS13:dev Feb 1, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for review and merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants