Collision mesh bounding boxes include non-collision meshes #3910

Open
jaj22 opened this Issue Jan 17, 2017 · 6 comments

Projects

None yet

4 participants

@jaj22
Contributor
jaj22 commented Jan 17, 2017

The Aabb (axis-aligned bounding box) built by CollisionVisitor currently includes all visible geometry, including shield meshes which extend well beyond the collision mesh. As far as I know, all usage is for physics purposes, so this feels like a bug. Last time I checked, only the radius was used for rendering. The Aabbs (and radius) also don't appear to take account of animation, although that's a much trickier problem and not an immediate one.

The docking AI routines assume that the Aabb describes the extent of a ship's collision mesh (which was true before the new model system), so the landing position is typically far too high, with ship dependence. This is currently worked around by extruding the collision mesh of the landing pads upwards, which would be unnecessary if the landing position was correct.

There are two main directions for a fix:

  1. tag_landing could be used instead of Aabb.min.y for determining landing positions, although this is more error-prone than using the true collision mesh extent, and requires modellers to add the tag for the game to function. If people do prefer this solution then I'd recommend removing the Aabb entirely, as it will have no useful functionality.

  2. Alternatively, changing CollisionVisitor to only include collision mesh geometry in the Aabb is a trivial function removal. However, the same Aabb radius is currently used for physical and render/clip radius (by different routes), so this functionality would need separation. Note that for performance reasons, physical and clip radius should ideally be separated even if tag_landing is used for the landing offset.

@nozmajner
Contributor

The point of tag_landing as far as I know is to indicate the lowest point of the lowered gears, which doesn't have animated collision meshes in most ships. Do the animated collision meshes taken into account for generating the bounding box?

@fluffyfreak
Contributor

@nozmajner is correct, it's to avoid using the collision bounding box for determining the lowest part of the ship which might not be the feet depending on animations etc. tag_landing is therefore just to position the ship a distance about the pad.

I don't think we take animated meshes into account when generating the AABB.

Removing the shield from the collision system could be good, honestly that whole thing was a fucking ball ache to work on. (pardon my language), It would be good to toggle collision with a shield on and off, it would also help the shield impact rendering effect to look better.

@jaj22
Contributor
jaj22 commented Jan 17, 2017

The point of tag_landing as far as I know is to indicate the lowest point of the lowered gears, which doesn't have animated collision meshes in most ships.

Are there currently any ships which do have animated collision meshes for landing gear? Because that would make this an immediate problem by breaking option #1.

@nozmajner
Contributor
nozmajner commented Jan 17, 2017 edited

@jaj22 : Lunar shuttle. And the Bowfin, buth the later has them only for the fins, not the gears.
The model viewer isn't showing them animating.

@mike-f1
Contributor
mike-f1 commented Jan 17, 2017

screenshot-20170117-212339

@nozmajner Lunar shuttle: but it works because of collision mesh aren't "retracted"

@jaj22
Contributor
jaj22 commented Jan 19, 2017

The lunar shuttle demonstrates a follow-on bug. Dynamic collision meshes are not currently being animated, either visually or for collisions. The code is there for it, but it doesn't appear to be working. Might be a simple bug, might not be.

Due to how mesh animation works in Pioneer, it's likely to be difficult to generate a correct bounding box at model-loading time for all cases. Therefore the Aabbs are not generally valid and option #1 is the only straightforward one. Models will need a valid landing tag, and reasonably central static collision geometry extending down to it. Nozmajner has fixed up the current models in #3912.

Not sure what to do in cases where a landing tag isn't provided, as there's no simple solution that will always make docking work. I'm inclined to use 0,0,0 as the landing offset, which should at least give visibly wrong behaviour and lead people to adding the landing tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment