ViewPos

[Major Cooke]

Pull Request

Adds class ViewPos to actors. Changing this via SetViewPos will offset the view of the camera. This takes into account pitch and roll by default when offsetting, and is operating via the actor's primary angles, NOT view angles - those are applied after.

ViewPos is null until SetViewPos() is first called. For security reasons, changing the view position can only be done through SetViewPos since this deals with manipulating an external object (set to read only).

X: +Forward | -Back
Y: +Left | -Right
Z: +Up | -Down

More details available in the link above.

[Major Cooke]

Sorry for the bump. I'm curious if there's any progress on this pull request? I've started work on a project that makes significant use of the related ViewAngles feature and it's been extraordinarily useful thus far. I'd love to be able to go further with it.

So there's two ways this can be handled, that I can think of.

The first is the Actor way (chosen for the PR).

• + Much easier to maintain
• + Every actor has its own set of positions so book keeping requirements is far less.
• - Adds onto every actor, increasing ram usage and save size.

The second is the Playerinfo way (which the first attempt was made with).

• + Doesn't expand upon actor, meaning minimal increase in save size and RAM usage.
• - Book keeping required for everything. External cameras, actors, views, you have to set up extra code in the event you want to use.

Additionally, there's one bug I haven't been able to squash out yet: seeing the body of the player through a view portal. I need help on this one.

-----

Ultimately what I need are opinions on how to handle this in the best way possible. Generally speaking, how would everyone else handle it? My first PR had it handled with PlayerThink() but that turned into a maintenance nightmare. I'm not even sure if I handled this part correctly in my very first attempt for dealing with offsets, or if it should've been done there at all.

So what I'd like to hear from fellow developers is how they would handle it - including when the camera isn't actually part of the player. (Help fixing the portal issue would also be massively appreciated but it's a cosmetic issue.) Input would be greatly appreciated.

[Caligari87]

My own opinion on some of these is probably null since I'm not well versed in engine development. That said:

From what I can tell, the former ViewAngles was implemented as a part of Actor. In my opinion, for consistency it'd make most sense for this to follow that same paradigm. It's just weird to think about needing to mix the two like

Code: Select all

A_SetViewAngle(viewangle * 0.9, SPF_INTERPOLATE);
player.SetViewPos(player.viewpos * 0.9, SPF_INTERPOLATE); // or however it ends up working

I understand the concerns about increased memory usage, but when does that imply basically freezing the core actor code to new features entirely? Are we past that point? Is there anything on the horizon that could ever reduce that footprint?

As for portals, if I recall correctly the same problem happens with the quaking code, does it not? Sure it's annoying but at worst it's just a visual edge case / quirk that modders need to be aware of, and there's plenty of those.

[Major Cooke]

Now I remember what the main concern is here.

As someone whose very much inferior on coding skills compared to Graf, I do not possess the vision of how he imagines it would be like - at least not entirely. I'd need to see more in-depth on how he would handle it so I can follow suit.

That's what this is primarily waiting on.

[Rachael]

What you can do is create an actor flag that explains if this is even needed, or not. If it isn't, then nothing is created. Remember to use a renderflag for this.

Actually, a flag really isn't needed if you just go for nullptr checks.

If it is, then you need to allocate new memory to a structure and create a pointer. Yes, you will have to use a class or struct for this. This allocation should be dynamic in order to reduce the effect that it has on the renderer.

All properties of this would go to this structure.

Remember also to check the actor destruction, that it checks this pointer and marks the newly allocated structure and deletes it if need be, when the actor will otherwise be destroyed.

[Major Cooke]

I actually think a flag would be necessary so you can shut it off without needing to destroy it, so the data can be preserved for instances where the player is still alive and intact. I'll give it a try.

I'm assuming it'll need to be done both on ZScript and engine side too. But I'm curious... In ZScript, why is it that some structs you can check for nulls on, but others you cannot?

[Rachael]

That I don't know - but - it is really important to be able to check for nulls on this one.

[Major Cooke]

Spoke with phantom, got answers about that. And more. Quoted with permission.

Structs are only nullable if they're passed by pointer
Only structs marked as native can be passes by pointer/reference, and they're always passed like that
And you can't instantiate them from ZScript AFAIK, it has to be done in the C++ code
I don't think dynamic creation would be feasible either, as ZScript doesn't have properties, and you can't make setting a flag or variable do anything other than setting it directly

[Rachael]

No you would definitely do the memory management on the C++ side.

[Major Cooke]

That was my plan to begin with as well, so I'll give it a shot.

[Major Cooke]

Wanted to make sure I'm doing this correctly for creation/destruction of a struct.

Code: Select all

struct FViewPosition
{
	DVector3 Offset;
	int Flags;

	FViewPosition() = default;
};

In actor.h:

Code: Select all

FViewPosition	*ViewPos;

And in AActor:OnDestroy():

Code: Select all

if (ViewPos != nullptr)
{
	delete[] ViewPos;
	ViewPos = nullptr;
}

Look correct so far? I've never done this before in all honesty. So if I'm missing something, please let me know.

Plan is to introduce the various flags for ViewPos into the struct for it directly since it's not needed anywhere else.

[Rachael]

That all looks good so far. When you need to create a new one -

Code: Select all

if (!ViewPos)
	ViewPos = new FViewPosition;

Graf should approve of this though - I don't know if he wants to pass this through the GC or if it's even worth that. If he wants to use the GC then I am sure he'll know how to invoke that, hopefully.

[Major Cooke]

Hell, if that's the easiest way to do it then I'm all for it - but if that requires GC, then I'm not entirely certain what to do with it so for now, I'll go with what you posted. Thanks!

[Cherno]

I assume this allows for proper leaning mechanics for stealth-focused games? If so, I hope this gets added soon.

[Caligari87]

It could certainly be used for that application yes, among others.