GameViewport is an opaque type

[david-lebl-adastra]

744 issue

[david-lebl-adastra]

Possibly many breaking changes within this commit create during hackathon night. Possible solution with less refactoring using opaque type with context bound, which will allow "export" of Size class methods: opaque type GameViewport <: Size = Size

[davesmith00000]

Hi @david-lebl-adastra!

Thanks for having another go! But actually... I preferred the deprecated version. 🙂

An opaque type A <: B = B is the same as type A = B, i.e. it is no longer an opaque type, it's just a type alias, which is why the export works. GameViewport is a Size wearing a disguise.

I find type aliases to be useful only in very limited cases ad I tend to avoid them because they're misleading, and hence I'm fairly allergic to the opaque type A <: B = A pattern.

The beauty of an opaque type (without the context bounds) is that you can't mistake it for something else at compile time because the compiler won't let you. With the context bounds, you lose all mechanical compiler assistance.

So if the opaque type is just a type alias, then I'd rather not have it at all and embrace the breaking change.

Does that make sense?

[hobnob]

Is there a case here for a type alias though? I'm thinking of situations where maybe your method takes a viewport and you want to be clear that's what you're expecting, rather than just any old size?

[davesmith00000]

Is there a case here for a type alias though? I'm thinking of situations where maybe your method takes a viewport and you want to be clear that's what you're expecting, rather than just any old size?

Right but a type alias doesn't help you there. You either want an opaque type (which is a type alias where you cannot replace it with any old Size), or your declare bankruptcy and ditch GameViewport in favour of Size, as we originally did here.

Unfortunately the only way to replace GameViewport with a Size as an opaque type, is to manually expose all the methods. Doable, but the question is do we care? (<--- This is probably the better solution! However...)

Generally I've found having GameViewport to be different from Size to just be a hassle. The only nice thing about it is that it has constructors for the various common screen resolutions.

[hobnob]

I'm a +1 for an opaque type here then... We don't have any specific need yet for differences between Size and GameViewport, but it's nice to have that option (an Islands Cape method for example), but I'm happy to be outvoted 🙂

[davesmith00000]

Thanks again @david-lebl-adastra, sorry about the back and forth. It isn't always easy to know what to do until you try. 😅

If it's ok I'll push another commit here to bring it inline with the discussion - unless you want to do it?

I think the work is to keep what you've done, but remove the type bounds, so that means:

1. Bringing back the methods that take GameViewport types, e.g. def topLeft(viewport: GameViewport): Point.
2. Remove the <: Size.
3. Fix anything that doesn't compile (might be nothing, I think you've done 90% if not all of the work already, in fact)

What say you?

[davesmith00000]

Added one commit. In the end, point (1) from my last comment was largely redundant, (2) was easy, and the only thing I added was a syntax extension method to convert Size instances to GameViewport if needed.

One thing that happens with opaque types is that if you do this:

def foo(x: Size): ...whatever
def foo(x: GameViewport): ...whatever

Those two methods actually have the same signature after type erasure, so you have to have one or the other. In the end I decided it made more sense for GameConfig to take a GameViewport and the Camera methods to work on Size.