Jebbs / DSFML

DSFML is a D binding of SFML
Other
95 stars 23 forks source link

Improve Packet's API regarding readType() #174

Open luke5542 opened 9 years ago

luke5542 commented 9 years ago

Instead of having several different names for read methods based on types, why not have just a single set of overloads on read based on the type and let the compiler decide? My thoughts are to use a similar declaration to how std.stream works:

void read(out byte x);

Perhaps even turn Packet into a full-on iostream and implement both InputStream and OutputStream. While this particular suggestion might not be the best right now (as the std.stream library is deprecated and they are looking for a replacement in Phobos), I still think at least having the improved declarations for read (and perhaps even write) would be a good step forward.

Jebbs commented 9 years ago

They were wrapped that way because that's how they were named in CSFML and I just never fixed it. My plan was to have overloads for read with different return types and write with different argument types, but I'm not against any of your suggestions either.

Maybe it might even make sense to use ranges when it comes to read and write?

luke5542 commented 9 years ago

Makes sense, though I find the different return types on read to be potentially awkward to deal with, but maybe I'm just not thinking of the right syntax that would be used.

My experience with ranges is somewhat limited, so I have no idea if this would work well at all, but my initial thought is that it wouldn't be the best way to go. I think that the best plan of action is to maintain consistency with whatever stream library is included in Phobos, be it the current implementation in std.stream or any new one that's added in. After all, maintaining consistency with the language's standard streaming API is what SFML tried to do in the first place.

luke5542 commented 9 years ago

Just a thought, but this would probably be a good change when #85 gets put through.

Jebbs commented 9 years ago

I was thinking about this today, actually. I'm leaning towards templates as being a good solution.

We could have something like read!byte() or similar, and if done right it could auto magically work for user defined types too.

luke5542 commented 9 years ago

Hmm... I like that idea, and it definitely saves on code compared to the std.stream's interface. I'm uncertain exactly how it would work for user-defined types, though, so what sort of mechanism were you thinking of to grab the byte data from non-primitives?

luke5542 commented 9 years ago

Having looked through DSFMLC, I now think that this might be easier, if only to make custom types easier to support, if all of Packet were done in D, rather than having to go between DSFML and DSFMLC. Unless I'm missing something that you were thinking of?

We could use std.bitmanip for the relevant hton*() and append() calls.

Jebbs commented 9 years ago

Well, maybe not for classes, but I think I can get it working out of the box for structs. I'm thinking we could read a void pointer and cast to the struct, and do something akin to the opposite for writing.

luke5542 commented 9 years ago

Yea, we could use sizeof to figure out how much data we need to grab, though what about nested types or types containing arrays? So:

struct A {
    int foo;
}

struct B {
    double bar;
    A[] sillyValues;
}

I'm thinking we would need more logic to handle all of these. We might just want to do what SFML has done and make the users extend the capabilities to suit their needs, rather than trying to write a full-on serialization library of our own.

Jebbs commented 9 years ago

Yep, you're probably right.

For the time being I'll just not worry about it and get It working for primitives and arrays.

luke5542 commented 9 years ago

At the least, that sounds like the best place to start.

Jebbs commented 9 years ago

Finishing up the last of the tutorials you did now, and I was reminded of a few things that were relevant to this.

For getting things to work out of the box, we can limit it to things that are built in types, strings, or have the trait isPOD. That should be good enough I would think.

Another improvement I thought of was with canRead. What a terribly named method. Due to the way that it is supposed to be used, I'm thinking that perhaps a read should throw an exception or otherwise fail in some way that doesn't involve that method.