sf::Packet Usage

[Strikerklm96]

Multiple things.
I found that making certain member functions accept packets was a really good way of making a function dynamic in the way it accepts data. So using a packet, a member function might take an integer, but later on, I could override it in a derivative class and make it take two floats. Are there reasons I shouldn't be doing this?

In the case I do that, I want to be able to make sf::Packet << and >> accept some other types. Is there a way to do this other than writing in the Packet.hpp file?

This code works for everything I have tried it with, but is this code safe/consistent with the way data should be written/read with the packet?

template<typename ...T>
Packet& Packet::operator >>(std::tuple<T...>& data)
{
    if (checkSize(sizeof(data)))
    {
        data = *reinterpret_cast<const std::tuple<T...>* >(&m_data[m_readPos]);
        m_readPos += sizeof(data);
    }

    return *this;
}
template<typename ...T>
Packet& Packet::operator <<(const std::tuple<T...>& data)
{
    append(&data, sizeof(data));
    return *this;
}

[Ixrec]

I found that making certain member functions accept packets was a really good way of making a function dynamic in the way it accepts data. So using a packet, a member function might take an integer, but later on, I could override it in a derivative class and make it take two floats. Are there reasons I shouldn't be doing this?

This sounds questionable to me because the idea of a polymorphic function is to have the same interface even if derived classes choose to have different implementations.  Having the same interface means the caller shouldn't have to know which derived class he's calling; they should all take the same arguments.  If each derived class is expecting different arguments for this function, then this function is failing to represent a common interface and something's probably wrong with the design.  You might as well get rid of the inheritance if you don't want your derived classes to behave like proper derived classes.

So, how did you end up with a polymorphic function that needs to take completely different arguments in certain derived classes?


The actual code seems fine to me, but I'm not that familiar with sf::Packet.

[Laurent]

Is there a way to do this other than writing in the Packet.hpp file?

The tutorial shows how to do it.

[Strikerklm96]

The tutorial shows how to do it.

Oops, I did the tutorial a while ago, and missed it I guess. Thank you.

This sounds questionable to me... So, how did you end up with a polymorphic function that needs to take completely different arguments in certain derived classes?

I know this forum isn't for general C++, but you asked, so here it is. The code you will see is probably the worst system I have ever written, but I can't come up with anything better, which is why I'm asking you if you know of a better way.

I am making a game, and have previously used Source SDK, which has a really nice and easy Input Output system between unrelated objects in the game. If you follow this link, and skip down to Outputs, you can read about how an output/input works, more or less. It's pretty short.
https://developer.valvesoftware.com/wiki/Inputs_and_Outputs
More importantly though: http://gamedev.stackexchange.com/questions/32722/interactions-between-game-objects


The way I interpreted the stack overflow post initially, I thought of this:

class IOBase
{
    virtual void input(sf::packet packet);
};
class Bomb : public IOBase
{
    void stop();
    void explode();
    void move(sf::Vector2f coordinates);
    void input(sf::packet message)
    {
        message >> string inputFunc;
        if(inputFunc == "stop")
            stop();
        else if(inputFunc == "move")
        {
            message >> parameter;
            move(parameter);
        }
        ect...
    }
};
class Door : public IOBase
{
    void open();
    void close();
    void setSpeed(float speed);
    //do the same thing, but for our functions
};

But then I thought, that might be slow if there are a lot of possible if statements to check. What if I could make it figure out what function to call while the game is loading.
That can be done using function pointers. Each Command object, when loaded, will figure out what function pointer to choose.

struct Command
{
        sf::packet message;
        FunctionPointer funcP;
};
//when the Command is loaded, there is an if block to determine what function pointer to use based on what was loaded from file, so we do the checks here instead of on the fly
//so later in code sometime after we have found target
(target.*funcP)(message);


class IOBase
{
    virtual void input1(sf::packet parameters);
    virtual void input2(sf::packet parameters);
    virtual void input3(sf::packet parameters);
};
class Bomb : public IOBase
{
    void stop();
    virtual void input1(sf::packet parameters)
    {
        stop();//now we don't have to go through a big if block, we know input1 is associated with stop();
    }

    void explode();
    virtual void input2(sf::packet parameters);//and this explode

    void move(sf::Vector2f coordinates);
    virtual void input3(sf::packet parameters);//and this move
};
class Door : public IOBase
{
    void open();
    virtual void input1(sf::packet parameters);

    void close();
    virtual void input2(sf::packet parameters);

    void setSpeed(float speed);
    virtual void input3(sf::packet parameters);

    void kill()//note, for example these cannot be called in the IO system
    void reset()
};
 

Keep in mind, there wouldn't be an input function for every possible function, just certain game play related ones, like to open a door, disable a trigger, or play a sound, but not kill, or reset in this case;

There are other problems now though:
1. The minimum number of functions IOBase has is determined by the class with the most callable functions.
2. If two classes have a function called "enable", then I have to make sure that they all use the same IOBase input function, which wouldn't be too hard, but it's sort of like book keeping, and that is just stupid.
3. You can accidentally send a message to a function that doesn't make any sense, like input3 to a door, but with a parameter of sf::Vector2f, which expects a float. But this isn't actually a problem, because when you are creating a map for the game, if you send a door a vector when it has been stated that you should send a float, that is obviously the map creators fault, aka me. When the sf::packet fails to extract the float, it could be logged as an error during runtime, and indicate what function went wrong, and the name of the object that received the packet that was incorrect. So then the mapper could fix it.




First option is simple but pretty crude, and possibly slow.
Second option is... weird, and bad.
There is another way I thought of, but I can't say it's much better than these.
Please just tell me i'm stupid and direct me to a better solution, if there is one.

[Ixrec]

My immediate reaction to that is...

But then I thought, that might be slow if there are a lot of possible if statements to check.  What if I could...

Premature optimization!

Unless you tried the if/else chain and then did some tests to prove that was noticeably slower, I would much prefer the "crude" but readable/maintainable if/else chain to this function pointer stuff.  With modern C++, most low-level optimizations that are worth doing are probably being secretly done by your compiler already.



Back to the general issue of polymorphic design, it's true that in a situation like this you want a very flexible type that can encompass all possible messages you might want to send.  But it should not be as general as sf::Packet, because that can store literally anything.  You should rather have an abstract base class for all messages that lets you at least specify one or two methods or members that all messages should have.  Surely there's at least one.

A very simple but very good example you could look at is sf::Event, which has one type member and then a union.  Every SFML program ever uses an if/else chain on the type (without performance problems!) to see what members of the union it should look at.

[Strikerklm96]

Yeah, I have a really bad habit of premature optimization. I will look at sf::Event to see how it works. Although, the sf::Packet means I can send lots of data of any kind! I will revert back to the "if else" thing I guess.

However, I would think this is a somewhat commonly desired design pattern, is there really not a better way?

[zsbzsb]

What more do you want? Either way you need to select the type of packet and computers really can't branch / select any faster than ifs. 

I would bet $100 that when you run a profiler your if statements won't even show up.

[Strikerklm96]

You are 100% right, it's the "crude" part I don't like. But it's not really bad, if I wanted something dynamic I should have used an interpreted language like python, and then I really would have optimization problems to deal with!

[zsbzsb]

You are 100% right, it's the "crude" part I don't like.

To be honest, using an enum instead of a string would probably make your code more readable and less error prone - not to mention the saved bandwith. 

[Ixrec]

Although, the sf::Packet means I can send lots of data of any kind!

If you put, say, a std::vector<char> inside your message class, then you can send literally any amount of any kind of data.  You can even use a union of several different kinds of std::vector<>s so you don't need to convert to/from bytes.

Of course, copying those containers might be expensive.  But odds are sf::Packet has to do something equally expensive.

Also note that sf::Packet is designed for sending data across networks (ie, potentially between machines on different architectures), so it has to worry about things like endianness and alignment.  This is of course wasted effort when you're sending messages around within a single program on a single machine, so for all you know you might gain performance immediately by switching to an sf::Event like class that doesn't bother with that stuff.  If you ever need to send the messages across networks later, you can then define conversion operators from your message class to/from sf::Packet.

However, I would think this is a somewhat commonly desired design pattern, is there really not a better way?

The better way *is* using your own abstract base class.  That gives you total control over exactly what restrictions/flexibilities you want your messages to have.  You can choose to require that every message should come with the sender's name or a screen position or a timestamp or be serializeable for logging or be convertible to sf::Packet or whatever.  You can choose whether you want the message body to be union of ints/chars/floats, a stringified JSON object, a vector<char>, an xml document, or any other highly-customizable format that happens to suit your messaging needs.

[Strikerklm96]

Ok, thank all of you for all your help!