-
Notifications
You must be signed in to change notification settings - Fork 16
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
hard_rock
method should be idempotent
#80
Comments
I think this is okay; however, currently most things don't actually care about this, just the attributes, so I am wondering what the use case is. If you do end up doing this, we should do the same with EZ, HT, and DT |
not sure what you mean about most things caring about the attributes, I want the x and (especially for hr) y position to be correct. I have a method (https://github.com/circleguard/circlecore/blob/master/circleguard/hitobjects.py#L30) which takes a hitobject. Some usages call it with an unmodified hitobject, and some call it with a I then call |
my proposal from long ago was to have hitobject class have the Mod as attribute with Mod being its own class. I do not like all the mod parameters being passed in one at a time. But i haven't looked at the code in months so feel free to ignore what I say. |
I generally believe functions should only take the state which actually matters to them. I thought that using a Mod object would make it hard to tell which mods actually influenced different areas of the code. For example, code that deals with the time of a hit object doesn't need most mods; just DT/HT. I do think that the current solution is really tedious, so I would be open to making a |
If you really desire the separation there could be different mod subclasses but I think treating all mods together is reasonable |
I agree that we should put all the mods together if we do group them. I think I err on the side of more state separation instead over too much grouping. Once the abstractions settle it is easy to group things back together, and keeping things split makes testing easy. It is much harder to break something up because the dependencies are hidden, and in tests you may need to provide more mocked state that actually required. In this case with mods, I don't think those are issues anymore |
I have a scenario where I want to ensure that a hitobject is its hard_rock version, but I can't control where that hitobject comes from, and so have no way of knowing if I am being passed an already hard_rock-ified hitobject, or a normal hitobject. The cleanest solution imo would be to prevent multiple applications from doing anything.
@llllllllll will implement this if you give the go-ahead, or maybe you have another solution in mind
The text was updated successfully, but these errors were encountered: