Open rekterakathom opened 1 year ago
Nice PR.
@diwako For more discussion.
I like this solution. It seems to run significantly faster in most situations, with the odd exceptions of terrain objects gathered from nearestTerrainObjects, which is used in some hiding and fleeing functions.
@diwako Let's have a discussion once you are back about this one. :)
Allright. Review completed. Only to half a year.
We like the concept and appreciate the thought that has gone into it. However we are suspect about the maintainability (see review time) and would like instead to implement non-macroed version in key positions. :)
Allright. Review completed. Only to half a year.
We like the concept and appreciate the thought that has gone into it. However we are suspect about the maintainability (see review time) and would like instead to implement non-macroed version in key positions. :)
The way I see it, partially implementing this PR w/o macros is the less maintainable version. That way you'll end up with various position commands across the codebase, whereas with this PR you get universal getters / setters for the most common position format with singular definitions.
The way I see it, partially implementing this PR w/o macros is the less maintainable version. That way you'll end up with various position commands across the codebase, whereas with this PR you get universal getters / setters for the most common position format with singular definitions.
Yeah, hard to argue against the complexity that those macros are simplifying.
When merged this pull request will:
Replace all instances of
getPos
with the appropriate position command, usuallyASLtoAGL getPosASL
. This is becausegetPos
is not a suitable command for getting an objects 3D position due to inconsistent behaviour and slow execution. In my tests, the new macro is almost always faster than getPos. On open terrain execution time stays the same, but indoors the new method is 3 to 4 times faster.Edit: Also added a macro alternative to
setPos
. This will set objects to AGL positions which is much more consistent.getPos
setPos
POSITIONAGL(object)
getPos
in favour of the new macroSETPOSITIONAGL(object,position)
setPos
in favour of the new macro