Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trap_R_inPVVS is not used #855

Closed
illwieckz opened this issue May 7, 2023 · 13 comments
Closed

trap_R_inPVVS is not used #855

illwieckz opened this issue May 7, 2023 · 13 comments

Comments

@illwieckz
Copy link
Member

Thetrap_R_inPVVS function is not used, I don't know if we want to keep it for potential modes being ported to Dæmon, but actually this is just something not used by Unvanquished.

@illwieckz
Copy link
Member Author

illwieckz commented May 7, 2023

I've just checked ioq3, Smokin'Guns, RTCW, ET:Legacy, q3rally, OpenJK and WorldOfPadman and while all have a trap_R_inPVS function like us, none of them actually define trap_R_inPVVS and then don't make use of it. UrbanTerror used ioq3 or proprietary q3 one so we are safe to say they didn't used trap_R_inPVVS, and mods like True Combat or ET:Fortress could not use it if the engine didn't have it. I seen that Spearmint don't have it either, which excludes Turtle Arena at the same time.

@illwieckz
Copy link
Member Author

illwieckz commented May 7, 2023

And none of XreaL, ET-XreaL, and RBQUAKE-3 have trap_R_inPVVS. Same for Qio and various OpenWolf trees I grepped.

@illwieckz
Copy link
Member Author

commit c761047
Author: Matthias Bentrup <***@***>
Date: Sun Nov 4 16:25:58 2012 +0100

Add visvis calculation and cgame trap.

Visvis is similar to vis, but while vis indicates if there is a direct
line of sight between two clusters, visvis indicates that there is an
indirect line of sight (i.e. there is a common point from where there are
lines of sight to both clusters).

The computation of visvis used here is not geometrically exact, but if
it differs from the definition given above, it's always the case that
it may indicate an indirect invisibility where none exists, so it is
still safe to use it for culling. In practise the differences are not
important.

I wonder what's for. Maybe we can make use of this feature for some interesting trick, but for now it's not used.

@slipher
Copy link
Member

slipher commented May 7, 2023

According to Git history, alien sense blips and old human radar could only detect entities in the PVVS.

@illwieckz
Copy link
Member Author

Ah, that's an useful feature then. I would like to have blips, at least as an option.

@DolceTriade
Copy link
Contributor

PVVS would also be useful for bot calculations to rank targets

@Viech
Copy link
Member

Viech commented May 8, 2023

If I remember correctly, PVVS in practice felt just random. PVS is already a crude outer approximation for the visible set: it's normal that it allows you to "see" through heavy walls. PVVS would be even larger and could extend to parts of a map that are hardly connected. I wouldn't be surprised if it incorporates most of a map, or an arbitrarily large portion of it. I see no good use for it, and it could be demanding computationally.

@DolceTriade
Copy link
Contributor

Note that PVVS is a one time computation just like PVS (at least that's how I understand it...)

@slipher
Copy link
Member

slipher commented May 8, 2023

I'd say delete it since if we want to do it again we should do it in the gamelogic without a trap call. There's already e.g. G_CM_inPVS

@Viech
Copy link
Member

Viech commented May 9, 2023

Note that PVVS is a one time computation just like PVS (at least that's how I understand it...)

It still produces a dense graph, possibly the initial computation (on every map load?) could produce a lag and the queries could be slower than for PVS.

@VReaperV
Copy link
Contributor

VReaperV commented Aug 2, 2024

This sounds like it could be useful to cull shadow-casting lights, but IIRC the old shadowmap code does something else to ensure correct culling. It's also probably easier to just cull light frustum or sphere (or possibly a parallelepiped for directional lights).

@VReaperV
Copy link
Contributor

VReaperV commented Oct 25, 2024

The trap call is slated for removal, see Unvanquished/Unvanquished#3157. The engine functionality is something I'll evaluate for shadowmaps.

@VReaperV
Copy link
Contributor

VReaperV commented Nov 10, 2024

Done in #1428. The remaining engine functionality is being tracked in #1209.

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

No branches or pull requests

5 participants