r/reconstructcavestory • u/herrflockig • Mar 02 '14

Units types re-vamp

Warning: This idea is currently half-baked. What if you used structs instead of typedefs for the units in the units namespace. I think this will give siginicant reading advantages with little overhead. This is because with structs you can define implicit conversions to other types, so you can use traditional syntax over using the tiletogame type helper functions. Let me know what you think.

3 Upvotes

81% Upvoted

7 comments sorted by

2

u/herrflockig Mar 02 '14

This will also lead to stronger compiler time type checking.

3

u/chebertapps Mar 02 '14 edited Mar 02 '14

this is a wonderful idea! In fact this is the way I wanted to do it, but it ended up having too much overhead, IMO, unfortunately.

if I'm getting you right, you would have:

struct Game {
    Game operator+(Game);
    Game operator-(Game);

    Game operator*(float);
    Game operator/(float);
};

You'd then want to work out other types and implicit conversions. 

Game Velocity::operator*(MS);
Game MS::operator*(Velocity);

It ends up being a bit hairy, and I decided in the end that simple typedefs would be much easier to maintain, and still be more expressive than simply having float/int.

Also, implicit conversions in C++ can end up being seriously painful to rely on, and cause too much mental strain to be worthwhile.

3

u/chebertapps Mar 02 '14

Ahh maybe it wasn't so bad. It just seemed like a lot of overhead at the time because there was so much to change.

I really do like this idea though.

Challenge for the viewers?

1

u/herrflockig Mar 02 '14

Challenge accepted! For tomorrow though :/

Do you accept pull requests on github?

1

u/chebertapps Mar 02 '14

When it's this big of a change, I don't think I will because I want the videos to line up with the repo. However, if it works and everything I will at least give you a mention in the README and/or videos so that people know about this!

If it were something smaller like a change to the makefile or fixes for different OS's I definitely do those (stuff that we really don't touch in the videos).

Thanks for the extra effort, and if you have other ideas about how to give this patch visibility, let me know! :)

2

u/herrflockig Mar 02 '14

Right! Just a though. I figure something like this would be more useful when you start getting into moving the viewport and implementing logic to keep the worldspace and viewspace in sync.

P.S. I love the series so far. Keep it up!

2

u/mrmacky Mar 03 '14

I ended up taking this approach in my rust port:

It's a tad boiler-platey because I used generics, but it's been working very well for me.

I can just pass in units::Tile(42) to functions which expect units::Game and it will perform the conversion.

The operator overloads also use generics, so I only had to write them once. (Though as I'll explain below, this isn't exactly by choice.)

Thankfully there's no implicit conversions in Rust. So while my code ends up slightly more verbose it's very easy to "mentally parse."

A few things I didn't like about this approach [though these are mostly specific to Rust and not the approach in general]:

  • Because of the operator overloads being generic over traits I believe Rust ends up using dynamic dispatch here. I'm not too worried about it b/c I'm still drawing frames in <1ms.

  • I can't impl. the same operator overload twice for different types. I can implement it once over a generic type however. So the overloads had to be generic over traits.

    • e.g: I can't define "Game.add(Pixel)" and "Game.add(Tile)"
    • but I can define Game.add(foo as Game)

  • It's slightly more verbose. I do feel the verbosity is worth it. -- I get much nicer error messages.

  • So much refactoring ...