realXtend / tundra

realXtend Tundra SDK, a 3D virtual world application platform.
www.realxtend.org
Apache License 2.0
84 stars 70 forks source link

EC_DynamicComponent is poorly named #667

Open Stinkfist0 opened 11 years ago

Stinkfist0 commented 11 years ago

If transcribed literally, EC_DynamicComponent means EntityComponentDynamicComponent. This is not in the line with how the rest of ECs are named. If and when the "EC" prefixes are dropped from the component type names (#523), we should come up with a better component type name.

jonnenauha commented 11 years ago

I guess in theory component names can be changed as we play with ID over the wire. But afaik txml stores and reads components by name? So changing any name would break stuff heavily? this is all we have in txmls :P

Stinkfist0 commented 11 years ago

Naturally when implementing support for the EC-free component type names, (deserialization) support for the deprecated EC-prefixed type names will be retained.

jonnenauha commented 11 years ago

Well yes that is of course expected. But you are talking about changing the whole name to for example "FreeData", and im saying this will be problematic as txml:s dont have component IDs. Changing the name would not load any old "EC_DynamicComponent" with the newly named component, even if its the same.

@cadaver I think we should add component id to txml:s to <component name="EC_DynamicComponent" id="xx"> so we can at some point start renaming ECs without breaking disk serialization/deserialization or the network sync.

If we add the comp id to the txml we can in ~3-6 months time start renaming what we want and expect existing scenes to have been ported :)

cadaver commented 11 years ago

Component ID is trivial to add and use that to instantiate proper component. One more thing is to consider script compatibility. If there will be a large number of places in code that must take into account the "legacy" names of components then IMO it's just better to either

jonnenauha commented 11 years ago

Yes EC_DynamicComponent is mainly used in scripts and changing the name (outside removing EC_ prefix) will break this. But I don't see this as a big problem, its a simple string search and replace operation to *.js files.

Stinkfist0 commented 11 years ago

I would think that Entity.cpp and IComponent.h & cpp are the only places that would require modifying + naturally all COMPONENT_NAME declarations. I have a plan in my head how this could be accomplished quite easily. I'll try to scribble it down in the near future.

antont commented 11 years ago

BTW for the record about the name & design of this EC, when I started the work on it it wasn't meant as a FreeData component at all. The idea was to enable defining new EC types from dynamic langs (py, js, lua and such) -- which means that the EC type is not defined at compile time in C++, but dynamically at runtime somehow. Hence the name. C# uses a a bit similar naming convention with the 'dynamic' keyword.

I still think that would be nice for usability reasons, for example if a script defines some new EC type for people to use, it would have the right fields in the EC editor like normal ECs do too. Now you have to know what attributes to add and do it manually which is extra work and error prone. Also those fully defined custom ECs would work normally in the network synchronization and not have the overhead that DCs have .. my EC_Swarm would be efficient :) . This is mostly relevant for things that users add manually, some EC_MyLightRig or EC_OurGameAITarget where level designers put colours and whatever parameters.

So the name 'DynamicComponent' was not meant for people to see even, just to be the implementation mechanism for such custom components which would show in the GUI and perhaps also in TXML with their own custom name (EC_MyThingFromScript). It’s a common way to do scripting support that in native code level there is a special type which can be configured from scripts.

Anyhow I've later figured that if we ever implement that mechanism e.g. CustomComponent is a better name for such a scripting API support class. Even better, if we just allow scripts to implement IComponent by e.g. subclassing, the whole component-whose-type-is-to-be-defined-in-script is not needed.

I think back then someone at Ludocraft misunderstood the name to mean FreeData, something where you can dynamically add and remove attributes on the fly, and jumped to compete the implementation like that and I didn't mind too much as it was a simple way to sort of reach a part of the goal and get forward to actually do stuff with scripts :)

And as many apps have proved, FreeData is useful .. IIRC someone said that they've used it in c++ app code too, or? Just for info that it's a different thing than what DC targeted. Both could ofc exist, allowing custom ECs from scripts and FreeData, for different purposes.

jonnenauha commented 11 years ago

@Stinkfist0 Yes that might be the case, but I think you are still just talking about dropping the EC_ prefix. Me and Lasse are talking about the issue you started which is renaming a component, not just the prefix.

Don't start renaming ECs before txml:s save and read component IDs instead of using the name as the identifier. We need to have the ID based thing in production for a while before we can safely start renaming components. Especially EC_DynamicComponent as many scripts will break if the name is changed (again not talking about removing the prefix, you can do that whenever you want :)

For what its worth I think DynamicComponent is a better name than FreeData or something like that. the EntityComponent_DynamicComponent part is a bit silly if you spell the whole thing out loud like that. Maybe it could be EC_DynamicStructure or something like that, or EC_DynamicData even. EC_Dynamic is not very descriptive either I guess :)

Stinkfist0 commented 11 years ago

@jonnenauha No, I'm hoping we can catch to birds with a same stone here - even without introducing component ID to the txml. But we'll see how it goes. I like the sound of DynamicData btw.

@antont does raise an interesting point here. It would be nice to have a component of which type name can be arbitrary:

entity.Component("EC_DynamicComponent", "GameData").Attribute("playerCount")

vs.

entity.Component("GameData").Attribute("playerCount")

or more shortly:

entity.gameData.Attribute("playerCount")

This however would require introducing the component ID to the txml, but no harm in that I think.

Oh, btw that example got me thinking: maybe it would be nice if dynamic attributes would register themselves to the component also as properties:

entity.gameData.playerCount
antont commented 10 years ago

we've been thinking of this in the WebTundra work now, could test implementing (the pong test case uses custom data in a DC)

Stinkfist0 commented 10 years ago

Also as a related note the component type IDs are nowadays present in the TXML.