Jump to content
The Dark Mod Forums
Sign in to follow this  
Hamlet

Possible problem related to constraints while loading objects

Recommended Posts

This is another report originating from a GCC warning, in this case the comparison of two different enumerators.
This works by converting each of the two enumerators into a numeric value, and comparing them instead.
In general, an enumerator is better used in a way that does not depend from its value at all. If the value is needed, constants are generally better suited.
The relevant one of the two warnings comes from idAF::Load() in game/AF.cpp (line 910), where a variable of enumerator type declAFConstraintType_t from framework/DeclAF.h:

typedef enum {
  DECLAF_CONSTRAINT_INVALID,
  DECLAF_CONSTRAINT_FIXED,
  DECLAF_CONSTRAINT_BALLANDSOCKETJOINT,
  DECLAF_CONSTRAINT_UNIVERSALJOINT,
  DECLAF_CONSTRAINT_HINGE,
  DECLAF_CONSTRAINT_SLIDER,
  DECLAF_CONSTRAINT_SPRING
} declAFConstraintType_t;

is compared with a variable of enumerator type constraintType_t from game/physics/Physics_AF.h:

typedef enum {
        CONSTRAINT_INVALID,
        CONSTRAINT_FIXED,
        CONSTRAINT_BALLANDSOCKETJOINT,
        CONSTRAINT_UNIVERSALJOINT,
        CONSTRAINT_HINGE,
        CONSTRAINT_HINGESTEERING,
        CONSTRAINT_SLIDER,
        CONSTRAINT_CYLINDRICALJOINT,
        CONSTRAINT_LINE,
        CONSTRAINT_PLANE,
        CONSTRAINT_SPRING,
        CONSTRAINT_CONTACT,
        CONSTRAINT_FRICTION,
        CONSTRAINT_CONELIMIT,
        CONSTRAINT_PYRAMIDLIMIT,
        CONSTRAINT_SUSPENSION
} constraintType_t;

The comparison of values from two distinct enumerator types should be avoided because it requires the enumerator modifications to be synchronised, but it's very tempting to add an element to one forgetting about the other.
If this comparison is really required, there should be a single enumerator type.
Judging from the enumerator names, this departure from synchronisation has already happened (I would except a DECLAF_CONSTRAINT_SLIDER to be equivalent to a CONSTRAINT_SLIDER, and instead it's compared to a CONSTRAINT_HINGESTEERING).
The two definitions are in different parts of the code and do not apparently share headers. But more to the point, for what I know these numerical values might be serialised, and changing them would break saved data.
My recommended suggestions would be:

  • if possible, manage to have a single enumerator for both uses
  • if not, abandon direct comparison and use a slower multiple-choice comparison function.

I provide a patch implementing the latter suggestion, which is expected not to break any serialised data, by not changing the enumerator values. Note that this function relies on the heuristic logic of my brain to match the elements from the two enumerators. Somebody with some understanding should cross check it.
The patch also addresses the other enumerator comparison, which is more subtle and is caused by the fact that enumerators defined in a template class yield one different enumerator type for each template instantiation.

enumCompare.patch.txt

  • Like 2

Share this post


Link to post
Share on other sites

I can see the issue of comparing two distinct, non-numerical variables.

 

However, what issue does it cause?


FM's: Builder Roads, Old Habits, Old Habits Rebuild

WIP's: Several. Although after playing Thief 4 I really wanna make a city mission.

Mapping and Scripting: Apples and Peaches

Sculptris Models and Tutorials: Obsttortes Models

My wiki articles: Obstipedia

Texture Blending in DR: DR ASE Blend Exporter

Share this post


Link to post
Share on other sites

However, what issue does it cause?

This is the kind of question hard for one like me with no experience in anything of The Dark Mod to answer. In fact, I was hoping some other reader could connect more dots.

My contemplation of the code tells me that "AF" is Articulated Figures, which I may think objects which are not rigid. In this sense, it makes sense to have constraints between the rigid parts of the figure, which is what both enumerators enumerate. The issue is shown in idAF::Load(idEntity *ent, const char *fileName), which suggests it's trying to load data from a file. It is in a double loop where constraints are deleted from a physics object. So I guess what could happen is that the wrong constraints are deleted and the object does not react like it should to being dragged around.

Share this post


Link to post
Share on other sites

Your correct in your interpretation. AF's are non-rigid bodies like corpses or ropes. The constraints are used to limit the rotation of the connected bodyparts. They are, however working as they should. As far as I can tell we don't have any issues there.


FM's: Builder Roads, Old Habits, Old Habits Rebuild

WIP's: Several. Although after playing Thief 4 I really wanna make a city mission.

Mapping and Scripting: Apples and Peaches

Sculptris Models and Tutorials: Obsttortes Models

My wiki articles: Obstipedia

Texture Blending in DR: DR ASE Blend Exporter

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Sign in to follow this  

×
×
  • Create New...