Remotery icon indicating copy to clipboard operation
Remotery copied to clipboard

Q: Create internal names for properties?

Open JCash opened this issue 3 years ago • 8 comments

I wonder if creating "internal" variable names is preferable, in order to avoid name clashes. When I started porting to this api, I immediately got a clash where I had to change my prop name to "EngineProps", since there already was a struct named Engine. The drawback is that I'd rather print "Engine" when presenting the properties.

    rmt_PropertyDefine_Group(Engine, "Engine properties");
    rmt_PropertyDefine_U32(FrameCount, 0, FrameReset, "# frames", &Engine);

An alternative would be just passing the name (not a pointer to a variable), allowing the macro to create internal names for the variable:

    rmt_PropertyDefine_U32(FrameCount, 0, FrameReset, "# frames", Engine);

E.g. the variables could be prefixed and end up like _rmt_prop_Engine and _rmt_prop_FrameCount.

What are your thoughts on this?

JCash avatar May 22 '22 07:05 JCash

Ofc, this complicates things a bit given that the group name is not explicitly used in the macros, but simply passed along with the variable arguments.

JCash avatar May 22 '22 07:05 JCash

A naming convention could fix this. So you have EngineProperties instead of Engine and in registration various patterns are searched for, like "ending in Properties", where it cuts the name to leave Engine.

Last thing I want to do, though, is add alternate macros for parented vs. non-parented. Maybe another one is every property has to be parented and we just introduc a dummy Root property group. That way you can do property prefixing and not need a naming convention.

dwilliamson avatar May 23 '22 21:05 dwilliamson

Yeah, I guess that's what I'll end up doing (the naming convention). Feels like the least hassle.

JCash avatar May 24 '22 09:05 JCash

There is always this option:

#include <stdio.h>

#define EXPAND(x) x
#define VA_NARGS_IMPL(_1, _2, _3, NARGS, ...) NARGS
#define VA_NARGS(...) EXPAND(VA_NARGS_IMPL(__VA_ARGS__, 3, 2, 1, 0))

#define JOIN2(x, y) x ## y
#define JOIN(x, y) JOIN2(x, y)

#define PROP2(prop, parent) puts(#prop " " #parent)
#define PROP1(prop) puts(#prop)
#define PROP(...) JOIN(PROP, VA_NARGS(__VA_ARGS__))(__VA_ARGS__)

int main()
{
    printf("%d\n", VA_NARGS(1, 2, 3));

    PROP(PropA, Parent);
    PROP(PropB);
}

Can be tested here: https://godbolt.org/z/bTxPc39n1

I explain it in a really old blog post: https://raw.githubusercontent.com/dwilliamson/b/623a4882561c0893e01488730896c6660794e257/Posts/2011-07-15-10-23-25.txt

dwilliamson avatar May 24 '22 10:05 dwilliamson

All properties will still have that prefix when presented in the Html page? (unless I'm mistaken?)

I think the easiest for me is to somehow add my own versions of the defines. E.g. a version of _rmt_PropertyDefine:

#define rmt_propName(name) JOIN(_rmt_p, name)

#define _rmt_PropertyDefine(type, name, default_value, flags, desc, ...) \
    rmtProperty rmt_propName(name) = { RMT_FALSE, RMT_PropertyType_##type, default_value, flags, #name, desc, default_value, __VA_ARGS__ };

And then set the parent like so:

rmt_PropertyDefine_U32(FrameCount, 0, FrameReset, "# frames", &rmt_propName(Engine));

JCash avatar May 24 '22 14:05 JCash

You could remove the _rmt_prop_ prefix before it gets used for the viewer, here: https://github.com/Celtoys/Remotery/blob/5d6fb518a8123369c881d6adfb2e48323a06993f/lib/Remotery.c#L9015-L9018

dwilliamson avatar May 24 '22 14:05 dwilliamson

That is true, and I think if I need to change the source anyways, modifying the "QueueAddToStringTable" part might be the easiest.

JCash avatar May 24 '22 14:05 JCash

For those interested, here's the patch that I apply each time I now update the Remotery.c. (I prefix our properties rmtp_):

diff --git a/engine/dlib/src/remotery/Remotery.c b/engine/dlib/src/remotery/Remotery.c
index 7514d8d84..b6f01e46f 100644
--- a/engine/dlib/src/remotery/Remotery.c
+++ b/engine/dlib/src/remotery/Remotery.c
@@ -9766,9 +9766,17 @@ static void RegisterProperty(rmtProperty* property, rmtBool can_lock)
             }
 
             // Calculate the name hash and send it to the viewer
-            name_len = strnlen_s(property->name, 256);
-            property->nameHash = MurmurHash3_x86_32(property->name, name_len, 0);
-            QueueAddToStringTable(g_Remotery->mq_to_rmt_thread, property->nameHash, property->name, name_len, NULL);
+/// DEFOLD
+            const char* name = property->name;
+            if (name[0]=='r' && name[1]=='m' && name[2]=='t' && name[3]=='p' && name[4]=='_')
+            {
+                name = name + 5;
+            }
+
+            name_len = strnlen_s(name, 256);
+            property->nameHash = MurmurHash3_x86_32(name, name_len, 0);
+            QueueAddToStringTable(g_Remotery->mq_to_rmt_thread, property->nameHash, name, name_len, NULL);
+/// END DEFOLD
 
             // Generate a unique ID for this property in the tree
             property->uniqueID = parent_property->uniqueID;

It is also good to consider the fact that since all properties are declared at the same "level" at global scope, you need to keep all property names unique. E.g a property "Count" would be a common name for a property, so you need to prefix it in some fashion. That however , doesn't necessarily present well in the gui, so I'll probably remove the name of the parent propery as well: Example scenario:

rmt_PropertyDefine_Group(rmtp_Particles, "Particles");
rmt_PropertySet_U32(rmtp_ParticlesCount 0, FrameReset, "# particles alive", &rmtp_Particles);

JCash avatar May 30 '22 09:05 JCash