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

Wraps "repel" and "magnetized" mods into a base class #19952

Closed

Conversation

o-dasher
Copy link
Contributor

This pull request aims to address code repetition between OsuModRepel and OsuModMagnetized mods by creating a base class CursorBasedHitObjectPositionMod (This naming seens odd but i couldn't think of anything else) which both mods inherit from.

@bdach
Copy link
Collaborator

bdach commented Aug 25, 2022

This was not done originally very intentionally. I didn't think that code reuse in this particular case worked well.

This PR is a little better in that respect, but there is so little shared code, that I question if this is even worth it.

@peppy
Copy link
Member

peppy commented Aug 25, 2022

It may make more sense if this is being done as a prerequisite to a new mod which is being worked on – is that the case?

@o-dasher
Copy link
Contributor Author

It may make more sense if this is being done as a prerequisite to a new mod which is being worked on – is that the case?

That's not the case, i just figured out that this could be simplified and did it for the sake of code quality and ease of use

@peppy
Copy link
Member

peppy commented Aug 25, 2022

I think in the interest of simplicity, let's keep these separate for now. If there's ever a third mod of the same variety, it may be time to consider combining some code, although i'm not sure inheritance is the correct path.

@peppy peppy closed this Aug 25, 2022
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.

3 participants