Pathshape Conversion Progress

#1 Azaezel, Thank you for taking the time to put this up as a branch for testing. I will be needing a 'pathshape' type object for a project I'm working on eventually, so I checked out that branch when you posted it in the bounty thread. I quickly realized that it would not be suitable for my needs and went back to my higher priority issues. I think the Gamebase changes that the resource requires must be a carry over from the porting process. There must be a better way in T3D to properly interpolate the positions on all clients (local or remote) without adding overhead to GameBase. I also questioned the inheriting from StaticShape. Would inheriting from TSStatic work or are there features of the PathShape that need the datablock and additional overhead provided by StaticShape and Shapebase? The PathShape implementation itself seems very good and could be modified to work in a couple of different scenarios. The script interface could be retained and hopefully expanded. I'd love to see a script selected 'attachment method' that opens the door to a purely physics based PathShape. It's also worth examining whether the dependencies created in the Player and Item classes could be removed yet still provide the same functionality. It just creates one more obstacle to migrating code to a component based system.

Would you be willing to entertain the idea of redesigning the resource instead of just 'fixing' it? If so, I'd move it to the top of my list because I'd like the opportunity to collaborate on it with you.

If anyone from the steering committee is reading this and has seen the resource, perhaps you could weigh in: Would the SC consider a pull request that made such changes to GameBase and SceneObject for a single object type?

#2 Potentially - but what changes are we talking about? Are the generically useful changes, or are thy so specific as to be useless to everyone except PathShape users? I'm not familiar with the resource myself.

EDIT: Also, it's exciting to see someone getting on to this. The lack of moving platforms has mystified and bemused many newcomers to Torque over the years.

#3 To address both at once: the core of the tech it's self isn't so much a shape following a path, that's pretty much just a modified version of the pathcamera already extant in the engine. It's precisely those alterations to Gamebase which allow for relative positioning that are the core augmentations to the engine. The same tech could be used, for instance (theoretically) to allow a player to stand on the hood of a car their buddy is driving, or on a truck, move around on it's flatbed. (and unfortunately, where the flaw lies at present.) The rest of that stuff is just augmentations for clarity, or ease of usage by a leveler.

edit: I can say there doesn't appear to be anything particularly apparent that explicitly requires usage of a staticshape over a tsstatic as far as parenting goes, provided there that we support the requisite hooks for pack/unpack update and the like there for interpolation purposes. I do have a few additional observations from throwing resolution attempts at it, but that may prematurely poison the feedback data by ruling something out that may have just been an oversight this end if taken one step further, so leaving that purposefully vague at present.

edit2: I did use the script_onadd callback for re-assigning the path for reloading a level (Which is the bit where it pops it over to the first node in the assigned path, for instance, rather than requiring the resource's extra function when initializing the server gamecore, for instance, so that'd need a rewrite if we forgo datablocks and thier associated pre-existing callbacks. Looks like most of the rest drops out, at least on a strip-out and compile attempt. Something like 4-5 errors after a conversion to a TSStatic derivation.)

As to a total rewrite, my general default is start with what folks have likely rolled in if they would have done so independently by now so as to avoid conflicts their end, and work from there on an as-needed basis. Not unlike that hitbox PR. (Also why if it's already a resource, I go there first.)

#4 Those changes sound fairly generic and useful, so if they were to be implemented at a standard we are happy with, I don't see why we wouldn't have a very serious discussion about it. Obviously other members of the community may have good arguments about accepting the changes, but from that overview, I think I personally would be in favour.

#5 Thanks for the clarification. I hadn't really considered extending the object parenting ability to other SceneObject derived classes, such as a flatbed vehicle. That's one good argument for leaving the code as is. I wasn't suggesting eliminating the relative positioning of attached objects or dropping any of the functionality of the resource. I'm more interested in exploring if all of that could be moved into the PathShape class itself so it could become a 'PlugNPlay' object type that could be added to or removed from a project build without requiring all of the core engine changes. I do agree though, that is a lot of work to replace functionality that is basically working as-is right now...
I'm not certain that what I'm suggesting is even possible. It's probably not practical from a value/time viewpoint.

#6 Ok, lets break it on down as far as how it works form a high-level perspective as-is:

1) The attachment process:

During a given object's processtick that supports attachment it determines if it is within a reasonable distance of an object which it can attach its self to ('stock', that means checking for a rInfo.object->getTypeMask() & PathShapeObjectType, which is where our interest in the actual pathshape pretty much drops out).(github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/T3D/player.cpp#L2171 + github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/T3D/player.cpp#L4617)

If so, at that point it attaches its self to a SceneObject derivation:
(github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/T3D/player.cpp#L4634 + github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/scene/sceneObject.cpp#L...) (Do note at this point it requires an additional bitmask insertion.)

2) The update process:
A) Pretty standard stuff,
github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/T3D/gameBase/gameBase.c... at 532/588 or thereabouts, we read in the relative position of an object to it's parent (what it's attached to, as opposed to what it's derived from.) Tracing the transforms gets us:
B)github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/scene/sceneObject.cpp#L...
C)github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/scene/sceneObject.cpp#L...
D)github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/scene/sceneObject.cpp#L...
E)github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/scene/sceneObject.cpp#L...

3)The interpolation process:
During supported class's interpolateTick (gamebase, player, et al), we call updateRenderChangesByParent()
(github.com/Azaezel/Torque3D/blob/pathshape/Engine/source/scene/sceneObject.cpp#L...) I can confirm via experimentation that that if (getClassName() == "Player") check is required or you get the same type of jittering singleplayer that you do multiplayer (or at least the same visual artifacting, if not necessarily from the same source), which strikes me as... well, spooky, frankly, pointing at the possibility of one point of research that I have yet to satisfactorily conclude.

Should pretty much cover the cliffs-notes version.

#7 Cheers for that. Off the top of the head:

1) I'd feel better about having actual attachment logic left to scripts. For example, maybe an onCollision callback should be used. I don't think Player should be specifically looking for PathShapes. I'd also object to the use of a hardcoded 1m raycast distance, but that's just me.

The SceneObject code looks decent, but I question the need for us to have a separate mounting system to the one that exists in SceneObject already. I realise it's only really used for ShapeBase mounting, and even then inconsistently, but I'd much prefer someone to extend that system and make it useful than implement another system beside it.

I do have some small niggles. First, the 'isChildOf' method is a misnomer, I think, as it seems to check for all descendents rather than just children. I prefer ancestor/descendent terminology. Likewise, 'mGraph' is a misnomer as I'm pretty sure the final structure is a tree (yeah, I'm a pedant :P). Also, this worries me:

deleteNotify(newParent); // if we are deleted, inform our parent

I'm pretty sure the semantics of this are the other way around, aren't they? I.e., if our parent is deleted, we will be notified. Which is probably as it should be.

2) Same story with the GameBase changes - I'd prefer us to actually network all the mounting there, instead of delegating that to subclasses as it's currently done IIRC. Also, the setProcessAfter business should be handled in the onNewParent callback that's defined in SceneObject, rather than happening directly in unpackUpdate.

3) Can all objects not just use getRenderTransform? That getClassName is horrible code smell.

#8 1) There are script calls, so tossing out the source's auto-attach should be perfectly doable. (as always accent on should till it's tested.)

1b) I gather the primary reason for differing terminology was fixed vs relative positioning clarity, though I can certainly see the draw there once we hit the cleanup phase for conflating the two systems, and standardizing usage. (also applies to point 2)

1c) agreed on clarifying ancestor/descendant in cleanup.

1d) mGraph read to me as a scenegraph. Those are also set up as trees conceptually for quite a few game engines. see en.wikipedia.org/wiki/Binary_space_partitioning for one example of where that was likely coming from subconsiously for the initial author(s).

3) Agreed. 'spooky' was not exactly the first phrase I used when encountering that particular function portion, but I do try and keep the colorful language muted a bit when it comes to forums at least. I suspect drilling down to why it is that player in particular is being treated as such a special snowflake in this circumstance would put us a long way towards a more homogeneous approach.

#9 Also note that the line

ParentObjectMask = BIT (4 ),

(added to sceneObject.h) breaks ProximityMine and AITurretShape because they both make use of 32 mask bits already.

This whole resource would break-down nicely into two object behaviors that could be used alone or combined. Path following and the ability to transfer relative movement to any objects that are climbing on or attached to a moving object. Is it worth trying to add this in a way that could be applied to all object types? I don't know the chances or time-frame for object behaviors making it into the engine.

TSStatic turned out to be a poor choice for a base class for a couple reasons. The packUpdate() call does not make use of the TransformMask or AdvancedStaticOptionsMask and it will assert if made a tickable object with no ambient animation. I'm still looking into ways to duplicate the resource functionality from an add-on class that doesn't require other engine changes.

#10 Thanks for the reminder. Now that that's been accepted into dev-head, freed up 2 bitmasks by rolling in the (first) shapebase cleanup, and rolled that one into the working branch. (Obviously, should this one get cleaned up functionally before behaviors make it in, a clean one would be made. Either way, resolving or sidestepping the interpolation error seems it'd be a primary goal for both methodologies.)