Skip to content

style: clean up redundant virtual#156

Merged
Pistonight merged 1 commit intozeldaret:masterfrom
TingleDinkle:cleanup/remove-redundant-virtual
Feb 16, 2026
Merged

style: clean up redundant virtual#156
Pistonight merged 1 commit intozeldaret:masterfrom
TingleDinkle:cleanup/remove-redundant-virtual

Conversation

@TingleDinkle
Copy link
Contributor

@TingleDinkle TingleDinkle commented Feb 15, 2026

Changes:

  • worldShootingStarMgr.h: removed virtual from init_() and calc_() which already have override
  • dmgStruct20.h: removed virtual from Struct20::combineMaybe() which already has override

This change is Reviewable

@Pistonight
Copy link
Collaborator

@TingleDinkle just curious, are you following some kind of list for these clean ups? The places you are changing and the type of clean ups seem a bit random. If possible you can do all of the clean up of the same category (like removing virtual) for the entire code base in the same PR. I think the commit history will be a bit cleaner that way

@TingleDinkle
Copy link
Contributor Author

@TingleDinkle just curious, are you following some kind of list for these clean ups? The places you are changing and the type of clean ups seem a bit random. If possible you can do all of the clean up of the same category (like removing virtual) for the entire code base in the same PR. I think the commit history will be a bit cleaner that way

So I just sorta dive into the codebase and click around and read then make changes where I see, no list. You're right that it is random because it absolutely is haha :)

Noted though, makes sense to do cleanups of the same nature in one commit. This is my first open-source project contribution so I'm getting used to these semantics.

@TingleDinkle TingleDinkle force-pushed the cleanup/remove-redundant-virtual branch from 84d23dd to a674acc Compare February 15, 2026 02:04
@TingleDinkle TingleDinkle changed the title style: clean up redundant virtual and SEAD_RTTI spacing style: clean up redundant virtual Feb 15, 2026
@TingleDinkle
Copy link
Contributor Author

I've seen no other instances of issues in this category (virtual redundancy) but in these 2 files. Should be good right?

@Pistonight
Copy link
Collaborator

Thanks @TingleDinkle, if you want to continue these clean-ups, a good place to start is by running clang-tidy. There are thounsands of warnings right now. Some are legitimated and others we want to disable the rule in .clang-tidy (usually because every instance of the warning is because it's needed for matching)

@Pistonight Pistonight merged commit 8e588c2 into zeldaret:master Feb 16, 2026
3 checks passed
@TingleDinkle
Copy link
Contributor Author

Thanks @TingleDinkle, if you want to continue these clean-ups, a good place to start is by running clang-tidy. There are thounsands of warnings right now. Some are legitimated and others we want to disable the rule in .clang-tidy (usually because every instance of the warning is because it's needed for matching)

Haha yeah, you caught me :) I'll definitely look into .clang-tidy rules and zip its complaining mouth where I can once I set it up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants