`hard_rock` method should be idempotent

[tybug]

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

[llllllllll]

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

[tybug]

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 hitobj.hard_rock hitobject.

I then call hard_rock on it if hr is enabled in the replay, which incorrectly reverts it back to its normal state if it was already hr, and correctly turns it to hr if it was normal before. I would prefer to be able to accept both and not require that only one type is passed in.

[jxu]

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.

61

[llllllllll]

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 ModSet object, which may just be bitmask internally, that represents a set of active mods.

[jxu]

If you really desire the separation there could be different mod subclasses but I think treating all mods together is reasonable

[llllllllll]

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