Horde3D
http://horde3d.org/forums/

enums and prefixes
http://horde3d.org/forums/viewtopic.php?f=8&t=830		 Page 1 of 1
Author:  jimbo [ 01.07.2009, 08:33 ]
Post subject:  enums and prefixes
I'm not a member of the fanclub for these prefixes in the enums, I think they obfuscate and clutter the simplicity of the engine:

Code:
struct ParticleEffectResData
{
   /*  Enum: ParticleEffectResData
         The available ParticleEffect resource accessors.

      E_ParticleEffect    - General particle configuration
      E_ChanMoveVel       - Velocity channel
      E_ChanRotVel        - Angular velocity channel
      E_ChanSize          - Size channel
      E_ChanColR          - Red color component channel
      E_ChanColG          - Green color component channel
      E_ChanColB          - Blue color component channel
      E_ChanColA          - Alpha channel
      PF_ParticleLifeMin  - Minimum value of random life time (in seconds)
      PF_ParticleLifeMax  - Maximum value of random life time (in seconds)
      PF_ChanStartMin     - Minimum for selecting initial random value of channel
      PF_ChanStartMax     - Maximum for selecting initial random value of channel
      PF_ChanEndRate      - Remaining percentage of initial value when particle is dying
   */
   enum List
   {
      E_ParticleEffect = 800,
      E_ChanMoveVel,
      E_ChanRotVel,
      E_ChanSize,
      E_ChanColR,
      E_ChanColG,
      E_ChanColB,
      E_ChanColA,
      PF_ParticleLifeMin,
      PF_ParticleLifeMax,
      PF_ChanStartMin,
      PF_ChanStartMax,
      PF_ChanEndRate
   };
};


My suggestion would be to split it up:

Code:
struct ParticleEffectResElems
{
   /*  Enum: ParticleEffectResElems
         The available ParticleEffect resource accessors.

      ParticleEffect    - General particle configuration
      ChanMoveVel       - Velocity channel
      ChanRotVel        - Angular velocity channel
      ChanSize          - Size channel
      ChanColR          - Red color component channel
      ChanColG          - Green color component channel
      ChanColB          - Blue color component channel
      ChanColA          - Alpha channel
   */
   enum List
   {
      ParticleEffect = 800,
      ChanMoveVel,
      ChanRotVel,
      ChanSize,
      ChanColR,
      ChanColG,
      ChanColB,
      ChanColA,
   };
};

struct ParticleEffectResParams
{
   /*  Enum: ParticleEffectResParams
         The available ParticleEffect resource accessors.

      ParticleLifeMin  - Minimum value of random life time (in seconds)
      ParticleLifeMax  - Maximum value of random life time (in seconds)
      ChanStartMin     - Minimum for selecting initial random value of channel
      ChanStartMax     - Maximum for selecting initial random value of channel
      ChanEndRate      - Remaining percentage of initial value when particle is dying
   */
   enum List
   {
      ParticleLifeMin = 808,
      ParticleLifeMax,
      ChanStartMin,
      ChanStartMax,
      ChanEndRate
   };
};



But I really think these enums should be changed to defines or consts altogether, some languages don't support enums so a binding needs
to define all these values as constants anyway.

I would rather see a more traditional:

#define H3D_ParticleEffectResElems_ParticleEffect 800
#define H3D_ParticleEffectResElems_ChanMoveVel 801
...

or even

const int h3d_ParticleEffectResElems_ParticleEffect = 800;
const int h3d_ParticleEffectResElems_ChanMoveVel = 801;
...
Author:  swiftcoder [ 01.07.2009, 13:35 ]
Post subject:  Re: enums and prefixes
jimbo wrote:
I'm not a member of the fanclub for these prefixes in the enums, I think they obfuscate and clutter the simplicity of the engine... My suggestion would be to split it up.
The primary issue is that there are names that appear in more than one parameter list, and the prefix allows one to cleanly differentiate between them.

I am not sure what the big deal is (but obviously it is a big deal, since you are the third person to bring this issue up). Enums like this are a convention for C++ software, and most other engines do the same:
Code:
// Ogre:
enum TrackVertexColourEnum {
         TVC_NONE        = 0x0,
         TVC_AMBIENT     = 0x1,       
         TVC_DIFFUSE     = 0x2,
         TVC_SPECULAR    = 0x4,
         TVC_EMISSIVE    = 0x8
     };

// IrrLicht:
enum ECOLOR_FORMAT
{
   ECF_A1R5G5B5 = 0,
   ECF_R5G6B5,
   ECF_R8G8B8,
   ECF_A8R8G8B8,
   ECF_R16F,
   ECF_G16R16F,
   ECF_A16B16G16R16F,
   ECF_R32F,
   ECF_G32R32F,
   ECF_A32B32G32R32F,
   ECF_UNKNOWN
};


Quote:
But I really think these enums should be changed to defines or consts altogether, some languages don't support enums so a binding needs to define all these values as constants anyway.
#defines for constants are just bad, and you won't see them recommended much these days, because of the way they fail to respect scoping rules, and overwrite tokens in inner contexts.

As for binding to other languages, many languages don't support #defines or const int's either, so I don't see what we would gain there. I mean, enums are just a convenient way to organise integer values, so you are free to bind them however you like:
Code:
# python
class ParticleEffectResData:
   E_ParticleEffect = 800
   E_ChanMoveVel = 801
   E_ChanRotVel = 802
   E_ChanSize = 803
   E_ChanColR = 804
   E_ChanColG = 805
   E_ChanColB = 806
   E_ChanColA = 807
   PF_ParticleLifeMin = 808
   PF_ParticleLifeMax = 809
   PF_ChanStartMin = 810
   PF_ChanStartMax = 811
   PF_ChanEndRate = 812
Author:  jimbo [ 01.07.2009, 15:07 ]
Post subject:  Re: enums and prefixes
The enums are surely the most elegant way to define the values in C/C++,
but current struct+enum has some disadvantages in my opinion targeting other languages than C++:

- SWIG can't cope with the duplicate names.
- It's using structs as a namespacing workaround.
- The struct definitions are in the global namespace, the functions in the Horde3D namespace.

Looking at the Ogre and Irrlicht enums, they prefix all values with the shortened "enum name" as well.

So this might be a possibility:

Code:
   /*  Enum: ParticleEffectResParams
         The available ParticleEffect resource accessors.

      H3D_ParticleEffectResParams_ParticleLifeMin  - Minimum value of random life time (in seconds)
      H3D_ParticleEffectResParams_ParticleLifeMax  - Maximum value of random life time (in seconds)
      H3D_ParticleEffectResParams_ChanStartMin     - Minimum for selecting initial random value of channel
      H3D_ParticleEffectResParams_ChanStartMax     - Maximum for selecting initial random value of channel
      H3D_ParticleEffectResParams_ChanEndRate      - Remaining percentage of initial value when particle is dying
   */
   enum H3D_ParticleEffectResParams
   {
      H3D_ParticleEffectResParams_ParticleLifeMin = 808,
      H3D_ParticleEffectResParams_ParticleLifeMax,
      H3D_ParticleEffectResParams_ChanStartMin,
      H3D_ParticleEffectResParams_ChanStartMax,
      H3D_ParticleEffectResParams_ChanEndRate
   };


It's also more simple to write a converter for this to another language.
Author:  marciano [ 03.07.2009, 08:20 ]
Post subject:  Re: enums and prefixes
I agree with swiftcoder. Enums are certainly more modern than constants or defines that have no type safety at all. I think it is just a bit nicer to have the enums wrapped in a struct than having the long enum names. It gives a better impression of grouping and is more suitable for code completion (like intellisense).
Code:
H3DMatData::E_Sampler
H3DMatData_E_Sampler

Regarding portability to other languages: Most modern languages have some way to express that "grouping". If some language does not, just use the group name as prefix with an underscore. I think it is not a problem if the API syntax is slightly different in different programming languages. Furthermore, the API is quite small and easy to port by hand, so automatic conversion using SWIG should not be the main argument for a style decision.

I agree that the prefixes won't win the price for the most beautiful syntax but I don't think that they clutter the API. They make clear which parameter can be used with which function (e.g. setParamI or setParamF), so they improve the clearness. Another option would be to put all different parameters in different enums (like MaterialDataFloatParams) but that gives even longer names and makes programming less convenient since you always need to know in which enum the desired parameter is.
Instead of using the prefixes, we could also choose a more verbose syntax like:

Code:
struct H3DMatData
{
   enum List
   {
      MaterialElem = 400,
      SamplerElem,
      UniformElem,
      MatClassParamF,
      MatLinkParamI,
      MatShaderParamI,
      SamplerNameParamS,
      SamplerTexResParamI,
      UniformNameParamS,
      UniformValueParam4F
   };


But that increases the length of the names again and I think the other variant is a bit clearer once you know what the prefixes mean.
Author:  marciano [ 03.07.2009, 19:54 ]
Post subject:  Re: enums and prefixes
Slightly different idea: Elements get the suffix Elem, streams the suffix Stream and parameters just get the type as suffix (I, F, Str).

Code:
struct H3DMatData
{
   enum List
   {
      MaterialElem = 400,
      SamplerElem,
      UniformElem,
      MatClassStr,
      MatLinkI,
      MatShaderI,
      SampNameStr,
      SampTexResI,
      UnifNameStr,
      UnifValue4F
   };
};


This makes clear what an enum member may be used for, the average name length is not much more and it looks a bit nicer than the prefix+underscore approach.
Author:  swiftcoder [ 03.07.2009, 20:22 ]
Post subject:  Re: enums and prefixes
marciano wrote:
Slightly different idea: Elements get the suffix Elem, streams the suffix Stream and parameters just get the type as suffix (I, F, Str)...

it looks a bit nicer than the prefix+underscore approach.
It may look nicer (not bothered personally), but it isn't nearly as easy to read.
Author:  jimbo [ 03.07.2009, 20:24 ]
Post subject:  Re: enums and prefixes
I like it! It also matches more with the function name scheme (getResourceParamI/F/Str etc).
Author:  marciano [ 04.07.2009, 22:18 ]
Post subject:  Re: enums and prefixes
Whatever naming convention we use for the resources should probably be taken over for the scene node params as well. In the end it is mostly an aesthetic thing, however I think it is worth spending some thougts on the decision (we want a nice, efficient and easy to use API for Horde :)). To summarize, we have the following options:

Code:
1 H3DEmitter::RespawnCount   (no type info, user has to estimate/know or look at docs)
2 H3DEmitter::I_RespawnCount
3 H3DEmitter::IRespawnCount
4 H3DEmitter::iRespawnCount     (similar to Hungarian notation)
5 H3DEmitter::RespawnCount_I
6 H3DEmitter::RespawnCountI

I would probably prefer 6. It is the option which makes the type info least dominant.
Author:  swiftcoder [ 04.07.2009, 22:43 ]
Post subject:  Re: enums and prefixes
marciano wrote:
Whatever naming convention we use for the resources should probably be taken over for the scene node params as well. In the end it is mostly an aesthetic thing, however I think it is worth spending some thougts on the decision (we want a nice, efficient and easy to use API for Horde :)). To summarize, we have the following options:

Code:
1 H3DEmitter::RespawnCount   (no type info, user has to estimate/know or look at docs)
2 H3DEmitter::I_RespawnCount
3 H3DEmitter::IRespawnCount
4 H3DEmitter::iRespawnCount     (similar to Hungarian notation)
5 H3DEmitter::RespawnCount_I
6 H3DEmitter::RespawnCountI
Option 3 is probably too similar to the C++ convention for naming abstract interfaces.
Quote:
I would probably prefer 6. It is the option which makes the type info least dominant.
On the flip side, the type information is pretty important - it isn't exposed in any other way for most of these.
Author:  marciano [ 05.07.2009, 12:53 ]
Post subject:  Re: enums and prefixes
swiftcoder wrote:
On the flip side, the type information is pretty important - it isn't exposed in any other way for most of these.
Yes but I think it is rather required as a hint. If you use Horde for some time you know what parameters have what type. Furthermore, it is quite clear for many parameters anyway (e.g. it is clear that names are string and something with "count" is int).

Actually I really like the suffix (without underscore) convention. It gives the required info and does not clutter the API's look&feel. So to bring that long discussion to a good end I have finally changed all the enums in the svn now. The struct names are shorter as well and everything got the prefix H3D.

EDIT:
Here is the updated documentation.
Feedback and suggestions are still possible until we bring out the API with the next point release.
Author:  jimbo [ 05.07.2009, 18:37 ]
Post subject:  Re: enums and prefixes
looks great to me, thanks. Only one issue, the getNodeParamF/setNodeParamF now take an extra argument compIdx,
is this only for the ColorF3 ? I like the H3DLight::Col_RI Col_GI Col_BI params better, you can easily set them independently and no need to clutter the float functions (the I variant doesn't have compIdx).
Author:  swiftcoder [ 06.07.2009, 11:56 ]
Post subject:  Re: enums and prefixes
jimbo wrote:
looks great to me, thanks. Only one issue, the getNodeParamF/setNodeParamF now take an extra argument compIdx,
is this only for the ColorF3 ? I like the H3DLight::Col_RI Col_GI Col_BI params better, you can easily set them independently and no need to clutter the float functions (the I variant doesn't have compIdx).
I kind of think a separate setNodeParam3f wouldn't go amiss for this situation. Would save the complaints from the Ogre guys ;)
Author:  marciano [ 07.07.2009, 21:22 ]
Post subject:  Re: enums and prefixes
jimbo wrote:
lOnly one issue, the getNodeParamF/setNodeParamF now take an extra argument compIdx,
is this only for the ColorF3 ? I like the H3DLight::Col_RI Col_GI Col_BI params better, you can easily set them independently and no need to clutter the float functions (the I variant doesn't have compIdx).

There is still another parameter that takes a vector and the resources have a few vector params as well. The reason that only the F functions have the component index is that only floats can have vector semantics. BTW, of course you can still set the color components idependently.

swiftcoder wrote:
I kind of think a separate setNodeParam3f wouldn't go amiss for this situation.

Yeah that would be an option as well. But what about getNodeParam3F? It would require pointers to store the values which is also not so nice, especially for non-C languages.
Author:  swiftcoder [ 07.07.2009, 21:44 ]
Post subject:  Re: enums and prefixes
marciano wrote:
swiftcoder wrote:
I kind of think a separate setNodeParam3f wouldn't go amiss for this situation.
Yeah that would be an option as well. But what about getNodeParam3F? It would require pointers to store the values which is also not so nice, especially for non-C languages.
I don't think it would be so bad - python/ctypes allows pointers to be passed very cleanly, at least.
Page 1 of 1 All times are UTC + 1 hour
Powered by phpBB® Forum Software © phpBB Group
https://www.phpbb.com/