Vec<2/3>Diff

[Major Cooke]

Pull Request

Introduces two new static functions that can be called from ui or playsim (clearscope) which calculates the difference between two points and factors in portals.

Because requiring two actors is not always ideal. ESPECIALLY from UI!

[Gutawer]

I don't think this works correctly. In a map without portals, all 3 of these methods should return the same value:

Code: Select all

class TestHandler : EventHandler {
	override void worldTick() {
		Actor player = players[consoleplayer].mo;
		Actor zombie = Actor(ThinkerIterator.create("Zombieman").next());
		Vector3 vec1 = player.vec3To(zombie);
		Vector3 vec2 = zombie.pos - player.pos;
		Vector3 vec3 = Actor.vec3Diff(player.pos, zombie.pos);
		Console.printf("----");
		Console.printf("%f, %f, %f", vec1.x, vec1.y, vec1.z);
		Console.printf("%f, %f, %f", vec2.x, vec2.y, vec2.z);
		Console.printf("%f, %f, %f", vec3.x, vec3.y, vec3.z);
	}
}

And yet, when I run this, I get this:
[imgur]https://i.imgur.com/g5RkNqd[/imgur]
Clearly, the bottom one is different to the others, which work fine.

[_mental_]

It will be great if you will provide a test for this PR. Anyway I decided to check it myself using this simple map.

Code: Select all

vector2 a = (-160, 0); // center of the first box
vector2 b = (160, 0); // center of the second box
vector2 d = actor.vec2diff(a, b);
console.printf("x = %.2f, y = %.2f", d.x, d.y);

The code above outputs x = 320.00, y = 0.00. I think x should be 128 in case of portal aware implementation because distance to portal line is 64 units in each box.
I know mostly nothing about portals so I might get it wrong.

[Major Cooke]

I have no idea how to set up interactive portals. That thing confuses the hell out of me. Someone else will have to do it.

Anyway, do you want me to change anything or not? Specifics please.

Currently addressing the requested changes in github.

[_mental_]

The problem is I don't have a test map in which new functions take portals into account.
Samples crafted quickly in SLADE confirm that they are exactly the same as vector subtraction no matter with interactive portals or without.
Maybe I'm missing something but what's the point of this change then?

[Major Cooke]

Addressed the param_prologue stuff.

Can you upload the sample for Gutawer and I to take a look at?

If that's the case, the portal awareness stuff in general might be broken... Does it happen if you have two actors that do the test across portals using Vec3to?

Anyway, the reason for this is because Vec2/3to requires the actors, and in UI context that's not acceptable. It needs to be able to pass in points and perform the portal calculations as well...

[_mental_]

If that's the case, the portal awareness stuff in general might be broken... Does it happen if you have two actors that do the test across portals using Vec3to?

It's the same. In any case sectors for both points share the same portal group.
This explains the same result as a vector subtraction. Don't ask me why it is so.

Also, P_PointInSector() and FDisplacementTable::getOffset() operate with 2D vectors.
Even if portal awareness is functional then resulting Z coordinate will be unaffected by it.
This point is quite easy to miss even after looking into source code.

Anyway, the reason for this is because Vec2/3to requires the actors, and in UI context that's not acceptable. It needs to be able to pass in points and perform the portal calculations as well...

I see but why did you put them in Actor class? There is no global functions in ZScript and LevelLocals seems to be the closest related class.

[Major Cooke]

Fixed and moved the functions.

[Gutawer]

Tested this with all the recent changes, it's working perfectly for me now with both vertical and horizontal portals.

[_mental_]

I forgot to chech how it works without a map loaded. Those sector pointers (and subsectors under the hood) worry me a bit. Can’t clarify this until tomorrow.
BTW the result is not obvious in this case. Probably it should be just v2 - v1 although it should not crash.

[Gutawer]

Good point. It hard crashes, which seems to be fixed by changing the return to something like this:

Code: Select all

if (sec1 == nullptr || sec2 == nullptr)
{
	ACTION_RETURN_VEC2(v2 - v1);
}
else
{
	ACTION_RETURN_VEC2((v2 + Displacements.getOffset(sec2->PortalGroup, sec1->PortalGroup)) - v1);
}

[Major Cooke]

Fixed.

[_mental_]

I'm about to push slightly modified version of this PR. Let me know if it's OK for you.

[Major Cooke]

Looks good to me.

[_mental_]

Added in c3c1e76.