Closed vittorioromeo closed 9 years ago
Hi, I'm glad you find this aspect of the library useful, it was an idea I didn't see anyone else do yet and seemed so natural so... yeah... :grinning:
As for your idea:
I initially experimented with static generation of synced objects (i.e. not requiring binding fields at runtime as is currently the case), but I figured that is simply not possible to implement in a way that is as intuitive to use as it is now. The idea is that all mutations of synchronized fields are automatically broadcast to whoever cares and that this mutation should be done in as natural of a way as possible, so as to not annoy/distract the programmer unnecessarily. C++ operator overloading is perfect for this, and I find that having to rely on generated setters and getters just makes SyncedType objects seem like "any other class" instead of an elementary object out of which you construct your more complex objects.
The only reason why the current constraint of having to register all fields with their owner exists is because C++ doesn't support reflection (yet?). The owner class simply can't "tell" what fields are part of it in order to collect/apply all changes when it is necessary, so the constructor is there to basically "register" the fields with their owner. Thankfully, since the class structure is static, we don't have to worry about de-registering anything.
This leads to the next problem I see when trying to implement something using bitsets. Your assumption is that the owning class owns the dirty bits of its fields, but in order to know how many are necessary at compile time, it has to be able to somehow "know" how many fields are in it at compile time, and as far as I know, this is not (currently?) possible. I am by no means a wizard in the black magic known as template metaprogramming (which I assume is necessary when expanding the parameter pack), so let me know if this is indeed possible (somehow I wouldn't be surprised if it were :grin:). This also doesn't solve the problem of generating a variable number of mutators/operators for all the fields based on compile-time information.
One can argue that having getters and setters that take a template parameter such as in your example could work, but personally, I don't find it very pleasing to the eye and can become very tedious if you have to access those fields a lot in your code (as you might be able to tell, I don't really like how one has to access std::tuple with std::get<>, but thankfully, a nicer alternative is coming as well). "Natural" operator overloading seems less intrusive if you ask me, and that is what the current concept was based around: A non-intrusive way of automatically synchronizing fields of data structures over multiple networked systems.
Don't get me wrong, your idea is good, but in order to not have to sacrifice many advantages I see the current system has in order to implement what you propose, I need more help from the language/compiler than is currently provided. This will probably come in the future. It could also be that I am overseeing some really dark art of pulling this off in a very elegant way, so please teach me if you know of any :wink:.
I am probably going to experiment a bit more with this, and will make any changes I feel don't compromise on the original "idea" of the system while of course optimizing wherever I can :wink:.
Thank you for your detailed feedback! I truly appreciate it. I've created a small proof-of-concept using my SSVUtils library:
#include <SSVUtils/Core/Core.hpp>
#include <SSVUtils/Json/Json.hpp>
class SyncObjBase
{
public:
virtual ~SyncObjBase() { }
};
template<ssvu::SizeT TI, typename TObj> class SyncFieldProxy
{
private:
TObj& syncObj;
public:
inline SyncFieldProxy(TObj& mSyncObj) noexcept : syncObj{mSyncObj}
{
}
template<typename T> inline void operator=(T&& mX)
{
syncObj.template setAt<TI>(ssvu::fwd<T>(mX));
}
};
template<typename... TArgs> class SyncObj : public SyncObjBase
{
template<ssvu::SizeT, typename> friend class SyncFieldProxy;
private:
static constexpr ssvu::SizeT fieldCount{sizeof...(TArgs)};
std::tuple<TArgs...> fields;
std::bitset<fieldCount> fieldFlags;
public:
template<ssvu::SizeT TI>
using TypeAt = std::tuple_element_t<TI, decltype(fields)>;
private:
template<ssvu::SizeT TI, typename T> inline void setAt(T&& mX)
{
fieldFlags[TI] = true;
std::get<TI>(fields) = ssvu::fwd<T>(mX);
}
public:
template<ssvu::SizeT TI> inline auto get() noexcept
{
return SyncFieldProxy<TI, decltype(*this)>(*this);
}
inline void resetFlags() noexcept { fieldFlags.reset(); }
inline auto toJsonAll()
{
using namespace ssvj;
ssvu::SizeT idx{0u};
Val v{Obj{}};
ssvu::tplFor(fields, [idx, &v](auto&& mI) mutable
{
v[ssvu::toStr(idx)] = ssvu::fwd<decltype(mI)>(mI);
++idx;
});
return v;
}
inline auto toJsonChanged()
{
using namespace ssvj;
ssvu::SizeT idx{0u};
Val v{Obj{}};
ssvu::tplFor(fields, [this, idx, &v](auto&& mI) mutable
{
if(fieldFlags[idx])
v[ssvu::toStr(idx)] = ssvu::fwd<decltype(mI)>(mI);
++idx;
});
return v;
}
};
struct TestPlayer : SyncObj
<
float, // X
float, // Y
int, // Health
std::string // Name
>
{
};
int main()
{
TestPlayer player;
player.get<0>() = 10.f;
player.get<1>() = 15.f;
player.get<2>() = 100;
player.get<3>() = "hello";
ssvu::lo("JSON_ALL") << "\n" << player.toJsonAll() << "\n";
ssvu::lo("JSON_CHANGED") << "\n" << player.toJsonChanged() << "\n";
player.resetFlags();
ssvu::lo("JSON_CHANGED") << "\n" << player.toJsonChanged() << "\n";
player.resetFlags();
player.get<2>() = 33.f;
ssvu::lo("JSON_CHANGED") << "\n" << player.toJsonChanged() << "\n";
player.resetFlags();
player.get<1>() = 11.f;
player.get<2>() = 33.f;
ssvu::lo("JSON_CHANGED") << "\n" << player.toJsonChanged() << "\n";
return 0;
}
The output:
[JSON_ALL]
{
"0": 10,
"1": 15,
"2": 100,
"3": "hello"
}
[JSON_CHANGED]
{
"0": 10,
"1": 15,
"2": 100,
"3": "hello"
}
[JSON_CHANGED]
{
}
[JSON_CHANGED]
{
"2": 33
}
[JSON_CHANGED]
{
"1": 11,
"2": 33
}
I appreciate that you have given this a lot of effort. :smiley:
However... like I said, these parts of your example are what I don't find fits into the "ideology" of SyncedObject
:
struct TestPlayer : SyncObj
<
float, // X
float, // Y
int, // Health
std::string // Name
>
TestPlayer player;
player.get<0>() = 10.f;
player.get<1>() = 15.f;
player.get<2>() = 100;
player.get<3>() = "hello";
There are multiple things that I find disconcerting with this model.
The first is that I am not really a fan of moving the data structure specification from the class declaration into the template declaration. It has been recommended by many other big names and I agree with them as well: Templates are meant as a way to generalize already existing structures/algorithms so they can be re-used more easily, a solution to C's lack of overloading and having to write the same function multiple times just with different parameter types.
Templates work well when the same semantics can be applied to the concept at hand. When you sort a container, you don't need to know what kind of a container it is, just that it is a container, hence the concept (this will even become a language feature at some point) of a container. When you want to construct a list of objects, it doesn't matter what kind of objects are in it. As long as they meet certain criteria, all list algorithms should work no matter what the element type is.
In your example, I find that your usage of variadic templates doesn't facilitate code re-use. There are many cases where they do (look at all the emplace methods and std::tuple as you already know) and I think it all boils down to the question of: "Do I use struct or std::tuple". I think this is a bit a matter of taste, and choosing the right tool for the job. The advantage of plain old structs is that you can access its members via a name, whereas you would have to access tuple members by their position which can become tedious if you start to have a large number of members (which is not uncommon if you think of use cases for SyncedObject). std::tuple shines when you need something that you tie into the interface of a function for example. It is essentially a container for static type information, being able to be used anywhere where traditional built-in types would typically be used. You wouldn't need to declare some random struct somewhere just to return it from a single function somewhere else like you would have in C or old C++ code, but this does force you to refer to the documentation to understand how to use what is being returned.
The second thing is that I personally don't find it very intuitive to have getters that return mutable references to the members of a data structure. I prefer having separate getters and setters, which even allows for optimizations in certain cases. I assume that you just tried to keep the example simple, and that in "real" code you would have getters and setters for all necessary members, but that shows that what you propose is just a lightweight proxy for a std::tuple data member in the SyncedObject class that you would access in the same way.
If I have over-interpreted what you are saying and all you are requesting is a simple space optimization, would you be able to make use of a partial template specialization like SyncedType<std::tuple<T...>>
that stores its dirty flags in a std::bitset
? That would in fact be relatively simple to implement.
Implemented in df1bb122824ca8d751903bf6cd753c53ecda3e7c.
It's not exactly what you suggested, but I think the core idea is the same. std::tuple
can now be used in sfn::SyncedType
and will make use of a single dirty flag just like any other type. This fits nicely into the philosophy behind SyncedType/SyncedObject without having to introduce new concepts into the library.
After a lot of tinkering, I came to the conclusion that adding flags for each element using a std::bitset
wouldn't really add much in terms of usability or performance. The current flag is also only used as an optimization instead of a necessity.
You will be able to access the tuple elements using std::get
on the contained value itself, which you can retrieve via .GetValue()
or .Get()
depending on whether you want a const or non-const reference to work with. It should be sufficiently demonstrated in the Synchronization example.
If you have any further suggestions, I'd be happy to hear them. :wink:
I'm a big fan of how easy to use your library is and wanted to share some thoughts about possible optimizations.
Instead of binding synchronizable fields at runtime, would it be possible to generate them at compile time using ´std::tuple´ and C++11 variadic templates?
I was thinking about something like this:
Generating the data structure wouldn't be too difficult. Do you, however, think that serializing and synchronizing structures generated that way would be possible?