Skip to content

+use on brush doors doesn't open them #1775

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

Closed
wgetJane opened this issue Feb 21, 2025 · 51 comments · Fixed by #1792
Closed

+use on brush doors doesn't open them #1775

wgetJane opened this issue Feb 21, 2025 · 51 comments · Fixed by #1792

Comments

@wgetJane
Copy link
Contributor

wgetJane commented Feb 21, 2025

  1. download example map: https://steamcommunity.com/sharedfiles/filedetails/?id=3183036324

  2. load map in ttt2

  3. try to press E on any door (rotating or sliding doors)

  4. it doesn't work

seems like a pretty severe issue

is there any reason why ttt2 needs to mess with the default +use behaviour on doors in the first place? seems like it just leads to a lot of breakage especially since it appears these changes barely get tested (if at all)

seems to be related to #1691 (from 3 months ago)

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

is there any reason why ttt2 needs to mess with the default +use behaviour on doors in the first place?

I'm not happy with this either. There's the issue with clientside interactions such as marker vision. This then caused issues which brought more fixes.

I'm almost considering reverting all of this. However, reverting fixes the issue with doors, but brings back some other old issues (like delayed opening of UIs when your ping is bad, no way of getting useful data from buttons etc)

This issue is also related to the issue with unusable vehicles on some maps

@TimGoll TimGoll added this to the Bugfix Backlog milestone Feb 21, 2025
@wgetJane
Copy link
Contributor Author

(like delayed opening of UIs when your ping is bad, no way of getting useful data from buttons etc)

those seem like much less severe issues than doors not working at all

it should at least be reverted for func_door and func_door_rotating for now

@wgetJane
Copy link
Contributor Author

wgetJane commented Feb 21, 2025

i think you shouldn't modify the default +use behaviour at all unless you really want to deal with all the headache that comes with it

you can still have the "Press E to use" stuff, not sure why you'd need to change how +use works at all

here's how you can replicate FindUseEntity clientside: https://github.com/ValveSoftware/source-sdk-2013/blob/master/src/game/shared/baseplayer_shared.cpp#L1067

im genuinely just very confused now that apparently this issue was known but was just left as-is because the loss of functionality for a bunch of doors in a bunch of maps was deemed a worthy tradeoff for something that's purely cosmetic

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

im genuinely just very confused now that apparently this issue was known but was just left as-is because the loss of functionality for a bunch of doors in a bunch of maps was deemed a worthy tradeoff for something that's purely cosmetic

At the time it wasn't know. It became clear after the release. But to our understanding it only affected a single map and vehicles on two maps. So we assumed it was ok. That it applies to ALL doors of that type wasn't clear to me. I tested it on so many maps, but apparently the wrong maps.

When I finished my vgui rewrite, I will redo the use handling. Because I'm unhappy as well. I think it might also be the reason the doorbuster stopped working.

Sidenote: I get that it is frustrating, but currently I'm happy if I have 4 or so hours per week to work on TTT2. I wish I'd could spend a day or two a week on TTT2. As much as I hated covid, I miss the days of non-stop coding for this project.

@wgetJane
Copy link
Contributor Author

actually i just realised, you dont need to replicate FindUseEntity clientside, because the targetid thing only shows up when you have the useable entity directly on your crosshair, and the game prioritises using entities on your crosshair

i'm really really confused now why this was done in the first place

@wgetJane
Copy link
Contributor Author

wgetJane commented Feb 21, 2025

im gonna gonna see if i can make a quick fix for it and hopefully there'll be a hotfix soon

edit: i spoke too soon, i did not know the extent of this

@wgetJane
Copy link
Contributor Author

wgetJane commented Feb 21, 2025

another thing: you can open doors by pressing E on stuff parented to doors (like glass), but ttt2 breaks this

(just check GetParent() too, or GetMoveParent() i dont remember exactly, i check it for my door-kicking code)

@wgetJane
Copy link
Contributor Author

ok reading more of the custom door code, i do not understand at all why the +use functionality was even tampered with

all the new fancy ui stuff is completely possible with networked vars with the default +use behaviour
(and preferably nw2vars rather than nw1vars to respect pvs and use less bandwidth)

delayed opening of UIs when your ping is bad

what ui are delayed? you can just override stuff that have a "clientside use" (like t buttons like base ttt does) and let normal +use fall through

also another thing i found: ttt2 breaks the "don't let crowbar open doors" setting

@wgetJane
Copy link
Contributor Author

im really just baffled that so much work went into trying to replicate +use for seemingly no reason, idk what to even think, im not even trying to be rude im just so confused

@wgetJane
Copy link
Contributor Author

wgetJane commented Feb 21, 2025

and like did nobody try to press E on a sliding door throughout the whole process of coding it

i returned to playing ttt2 since like a week ago and i'm immediately finding all these issues

@wgetJane
Copy link
Contributor Author

wgetJane commented Feb 21, 2025

@TimGoll can you give like a brief summary of the use + door stuff and what issues there were when trying to revert it, i wanna know if im going insane or not

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

im really just baffled that so much work went into trying to replicate +use for seemingly no reason, idk what to even think, im not even trying to be rude im just so confused

I don't want to shift the blame to someone else here, so don't get me wrong. Someone else in the team made a light rework of the use. That broke a lot of things. I thought I could fix this and dug a deep hole over multiple PRs until I addressed all issues (seemingly). At some point I should have made the call to revert all of it, but I assumed I could just fix it.

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

and like did nobody try to press E on a sliding door throughout the whole process of coding it

i returned to playing ttt2 since like a week ago and i'm immediately finding all these issues

Probably different map pool then my testing set. Now that I think of it, I don't have any map with sliding doors in it. And besides you, nobody really bothered reporting anything. I don't play much myself and in the YT videos I've seen of people playing TTT2, none of these issues arose.

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

can you give like a brief summary of the use + door stuff and what issues there were when trying to revert it, i wanna know if im going insane or not

Here's the original PR that caused the issues:
#1447

Here is my list of PRs that attempted to fix it:
#1580
#1604
#1605
#1606
#1608
#1610

@wgetJane
Copy link
Contributor Author

i could try to spend maybe next weekend trying to fix up the door stuff (zero guarantees, looks like hell)

the way i see it, ideally:

  • restore default +use functionality other than for stuff that needs to override it (c4, player corpses, t buttons, etc)

  • hook into when doors/buttons open/close, for networking their state for the ui

  • have some networked flags for the door type (usable door, touchable door, destructible door, etc)

  • make the targetid work when you point at an entity parented to a door/button (for glass doors and such)

  • maybe fix the crowbar stuff

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

(c4, player corpses, t buttons, etc)

the new marker vision (radio for example) uses it as well, just fyi

have some networked flags for the door type

the door stuff is completely independent of this. You can revert everything and the doors will still have working targetID. The issue are buttons because some flags had to be changed so clientside traces could hit them.#

make the targetid work when you point at an entity parented to a door/button (for glass doors and such)

that would be awesome!

@wgetJane
Copy link
Contributor Author

do you remember the maps you used to test with? maybe just the ones with the buttons/levers/whatever

@TimGoll
Copy link
Member

TimGoll commented Feb 21, 2025

  • elevators and stairs
  • community pool
  • some generic TTT maps with normal prop doors
  • a bunch of MC maps like jerome, island, ...
  • the maps mentioned in the issues/PRs, I hope they are mentioned at least

I want to reiterate that the issue is with buttons, not doors. Doors have issues (like the parenting), but the changes were introduced many years ago and work perfectly fine. The issue is with the use change in the aforementioned PR.

@Crashington
Copy link

i just found a door on one of my maps that didnt work in ttt2, likely because of all of this. But i managed to very easily fix it. I noticed in Hammer that apparently I had accidentally placed the origin of the func_door pretty far away from the actual door brush. moving the origin back into the brush fixed the door for me and made it fully usable again. I dont know if this is THE thing that causes this or just one out of many, but maybe it can help investigate this

@TimGoll
Copy link
Member

TimGoll commented Feb 26, 2025

I noticed a similar thing with the waterworld traitor room exit button. I think - while our implementation is not perfect - many of these issues arise because of bugs in maps. Like on the simpsons map where doors are buttons ...

@MrXonte
Copy link
Contributor

MrXonte commented Feb 27, 2025

i just found a door on one of my maps that didnt work in ttt2, likely because of all of this. But i managed to very easily fix it. I noticed in Hammer that apparently I had accidentally placed the origin of the func_door pretty far away from the actual door brush. moving the origin back into the brush fixed the door for me and made it fully usable again. I dont know if this is THE thing that causes this or just one out of many, but maybe it can help investigate this

That sounds very likely to be the issue. I am mapping for ttt/2, and I only use brush doors, but I only experienced this issue once, which was fixed after just making the door from scratch again. Will have to test to be sure, but it would make sense if it references to the origin.

I also had an "issue" i turned into a feature where a oneway door can still be used both ways with the crowbar, but not with use. Its a door on the one side, with a func_brush parented to the door on the other to block use. You cant +use open the door through the brush, but you can crowbar it open somehow.

@wgetJane
Copy link
Contributor Author

the way i see it, doors/buttons/etc behaving differently on use in ttt2 vs ttt is the main issue

@Crashington
Copy link

i just found a door on one of my maps that didnt work in ttt2, likely because of all of this. But i managed to very easily fix it. I noticed in Hammer that apparently I had accidentally placed the origin of the func_door pretty far away from the actual door brush. moving the origin back into the brush fixed the door for me and made it fully usable again. I dont know if this is THE thing that causes this or just one out of many, but maybe it can help investigate this

That sounds very likely to be the issue. I am mapping for ttt/2, and I only use brush doors, but I only experienced this issue once, which was fixed after just making the door from scratch again. Will have to test to be sure, but it would make sense if it references to the origin.

I also had an "issue" i turned into a feature where a oneway door can still be used both ways with the crowbar, but not with use. Its a door on the one side, with a func_brush parented to the door on the other to block use. You cant +use open the door through the brush, but you can crowbar it open somehow.

this is offtopic to the actual thread but check if you have a ttt_map_settings entity on the map and have "crowbars open doors" enabled. idk why this setting exists or is default on but its probably the issue

@TimGoll
Copy link
Member

TimGoll commented Feb 27, 2025

I think you can also override this setting in TTT2, but it would probably be best to fix it in the map

@wgetJane
Copy link
Contributor Author

afaik ttt2 doesnt respect your settings in ttt_map_settings, ttt2 allows me to open stuff with the crowbar on maps that i know have that disabled

@Crashington
Copy link

i definitly know that map settings have an impact in ttt. i assume any server level setting just overrides it.

@MrXonte
Copy link
Contributor

MrXonte commented Feb 27, 2025

afaik ttt2 doesnt respect your settings in ttt_map_settings, ttt2 allows me to open stuff with the crowbar on maps that i know have that disabled

The setting only sets if the crowbar unlocks openables, not if it opens them. The crowbar will always try to open if the server convar is set, but the setting determines if it unlocks as well.
Image
So the door will always open if in ttt_map_settings its set to unlock OR if it has Use Opens (flag 256) set.

@MrXonte
Copy link
Contributor

MrXonte commented Feb 27, 2025

also i am noticing how bad the naming scheme is since ttt_crowbar_unlocks convar manages if the crowbar tries to open doors, while the ttt_map_settings manage if if also tries to unlock them

will open a separate issue about this #1779

@wgetJane
Copy link
Contributor Author

wgetJane commented Mar 7, 2025

@TimGoll from my testing, certain buttons like momentary_rot_button just don't have their classnames sent to the client

can you tell me which maps you used for testing the button targetids?

@TimGoll
Copy link
Member

TimGoll commented Mar 7, 2025

@wgetJane Uhm, this has been a while. On the top of my head I think of the two or three clue map versions. Or the T-Tester in MC Island? But this one could be a door that is used as a leaver.

@Crashington
Copy link

This is something i forgot to mention outside of discord that is likely connected to this: In TTT2 you can for some reason press E/Use on func_rotating to make it pause or resume its rotation... this is not a feature the entity has outside ttt2 and cant be disabled in hammer or anything

@wgetJane
Copy link
Contributor Author

wgetJane commented Mar 7, 2025

This is something i forgot to mention outside of discord that is likely connected to this: In TTT2 you can for some reason press E/Use on func_rotating to make it pause or resume its rotation... this is not a feature the entity has outside ttt2 and cant be disabled in hammer or anything

yeah i'm aware of this, i've fixed this as well on a wip branch i have, i'll see if i can make a pr for it in a few days for testing

@wgetJane
Copy link
Contributor Author

wgetJane commented Mar 7, 2025

@TimGoll btw would it be fine to change the door and button nwvars to nw2vars? nw2vars respect pvs while nwvars will keep broadcasting updates to all players which is pretty overkill for something like door states and such

@TimGoll
Copy link
Member

TimGoll commented Mar 7, 2025

@TimGoll btw would it be fine to change the door and button nwvars to nw2vars? nw2vars respect pvs while nwvars will keep broadcasting updates to all players which is pretty overkill for something like door states and such

Probably yes. Altough there are some addons like the door locker that render a "thermal vision" for all doors on the client - would this break it?

I think you can split it in two parts: The existence of the door has to be known to every client (I think that doesn't use NWvars anyway). The info on specific door states is only relevant in PVS, yes.

@wgetJane
Copy link
Contributor Author

wgetJane commented Mar 10, 2025

should i just remove this?

net.Receive("TTT2SyncDoorEntities", function()
local amount = net.ReadUInt(16)
local doors = {}
for i = 1, amount do
doors[i] = net.ReadEntity()
end
door_list.doors = doors
---
-- @realm client
hook.Run("TTT2PostDoorSetup", doors)
end)

this doesnt make sense because most of these doors would be invalid to the player at this point due to pvs, this just gives the client a list of mostly invalid entities

if someone wants to keep track of doors clientside, they can just use OnEntityCreated

this is what `PrintTable(door_list.doors)` outputs on the client:
[1]	=	Entity [582][func_door]
[2]	=	Entity [583][func_door]
[3]	=	Entity [607][func_door]
[4]	=	Entity [609][func_door]
[5]	=	[NULL Entity]
[6]	=	Entity [1141][func_door_rotating]
[7]	=	[NULL Entity]
[8]	=	Entity [1175][func_door_rotating]
[9]	=	Entity [1176][func_door_rotating]
[10]	=	Entity [1177][func_door_rotating]
[11]	=	[NULL Entity]
[12]	=	Entity [1179][func_door_rotating]
[13]	=	Entity [1180][func_door_rotating]
[14]	=	[NULL Entity]
[15]	=	[NULL Entity]
[16]	=	Entity [1201][func_door_rotating]
[17]	=	Entity [1202][func_door_rotating]
[18]	=	Entity [1204][func_door]
[19]	=	Entity [1205][func_door]
[20]	=	[NULL Entity]
[21]	=	[NULL Entity]
[22]	=	[NULL Entity]
[23]	=	Entity [1216][func_door_rotating]
[24]	=	Entity [1217][func_door_rotating]
[25]	=	Entity [1218][func_door_rotating]
[26]	=	Entity [1219][func_door_rotating]
[27]	=	Entity [1231][func_door_rotating]
[28]	=	Entity [1237][func_door]
[29]	=	[NULL Entity]
[30]	=	Entity [1244][func_door_rotating]
[31]	=	[NULL Entity]
[32]	=	[NULL Entity]
[33]	=	[NULL Entity]
[34]	=	[NULL Entity]
[35]	=	[NULL Entity]
[36]	=	[NULL Entity]
[37]	=	Entity [1310][func_door]
[38]	=	Entity [1311][func_door]
[39]	=	[NULL Entity]
[40]	=	[NULL Entity]
[41]	=	Entity [1343][func_door_rotating]
[42]	=	[NULL Entity]
[43]	=	[NULL Entity]
[44]	=	[NULL Entity]
[45]	=	Entity [1348][func_door_rotating]
[46]	=	Entity [1349][func_door_rotating]
[47]	=	[NULL Entity]
[48]	=	[NULL Entity]
[49]	=	Entity [1384][func_door]
[50]	=	Entity [1389][func_door]
[51]	=	Entity [1392][func_door]
[52]	=	Entity [1393][func_door]
[53]	=	Entity [1467][func_door_rotating]
[54]	=	Entity [1468][func_door_rotating]
[55]	=	[NULL Entity]
[56]	=	[NULL Entity]
[57]	=	Entity [1471][func_door_rotating]
[58]	=	Entity [1472][func_door_rotating]
[59]	=	[NULL Entity]
[60]	=	Entity [1474][func_door_rotating]
[61]	=	Entity [1475][func_door_rotating]
[62]	=	[NULL Entity]
[63]	=	[NULL Entity]
[64]	=	Entity [1481][func_door]
[65]	=	Entity [1482][func_door]
[66]	=	[NULL Entity]
[67]	=	[NULL Entity]
[68]	=	Entity [1505][func_door]
[69]	=	[NULL Entity]
[70]	=	Entity [1920][func_door]

this doesn't seem it has any use case at all

@wgetJane
Copy link
Contributor Author

the simpsons map where doors are buttons

Image

@TimGoll
Copy link
Member

TimGoll commented Mar 10, 2025

should i just remove this?

TTT2/lua/ttt2/libraries/door.lua

Lines 231 to 245 in b00d445

net.Receive("TTT2SyncDoorEntities", function()
local amount = net.ReadUInt(16)

 local doors = {} 

 for i = 1, amount do 
     doors[i] = net.ReadEntity() 
 end 

 door_list.doors = doors 

 --- 
 -- @realm client 
 hook.Run("TTT2PostDoorSetup", doors) 

end)
this doesnt make sense because most of these doors would be invalid to the player at this point due to pvs, this just gives the client a list of mostly invalid entities

if someone wants to keep track of doors clientside, they can just use OnEntityCreated

this is what PrintTable(door_list.doors) outputs on the client:
this doesn't seem it has any use case at all

I'm hesistant on removing this because I know at least one of my addons uses this, maybe other addons as well. The invalid entities are all "door" entities that don't exist on the map. If you teleport to them, they are on weird positions outside of the playable area. Therefore I can say that the list seemed useful to me. If I highlight all those entities you listed this seems to match all doors usable on a map

@wgetJane
Copy link
Contributor Author

i mean removing the TTT2PostDoorSetup function on the clientside

at the point it is called, most of the table will be filled with null entities (these do not correspond to any entity) because the client has not seen those doors yet

i cant see a possible use case for wanting to receive such a strange list on the client

@wgetJane
Copy link
Contributor Author

@TimGoll can you link the addon?

@TimGoll
Copy link
Member

TimGoll commented Mar 10, 2025

@TimGoll can you link the addon?

https://steamcommunity.com/sharedfiles/filedetails/?l=german&id=2115945573

When having this weapon selected, it should show all doors on the map

@TimGoll
Copy link
Member

TimGoll commented Mar 10, 2025

i mean removing the TTT2PostDoorSetup function on the clientside

at the point it is called, most of the table will be filled with null entities (these do not correspond to any entity) because the client has not seen those doors yet

i cant see a possible use case for wanting to receive such a strange list on the client

I think this hook is unused so far, not sure. So it should be possible to safely remove this hook

@wgetJane
Copy link
Contributor Author

wgetJane commented Mar 10, 2025

just found it:

https://github.com/TTT-2/ttt2-wep_doorlocker/blob/23a63deb9c3893a52f281ea3c8a02b5cf8a11eef/gamemodes/terrortown/entities/weapons/weapon_ttt_doorlocker.lua#L127

this usage is not ideal

this assumes that door.GetAll() will give a list of all doors, but this is not the case, it will give a list of mostly null entities, as shown by my comment above #1775 (comment)

the null entities in the list will stay as null entities until the next round start, the potential non-null entities in the list will be the doors that were within the player's pvs when the round started

(funny note: the position used for that pvs might be where the player was right before they respawned, rather than where they respawned at round start, for the same reason why players will pick up weapons right before they respawn Facepunch/garrysmod#1887 )

the player will receive more door entities as they walk around the map after round start, however door.GetAll() will not be updated with these, again: the null entities in the list will stay as null entities until the next round start

a better way to implement door.GetAll() clientside would be something like this:

function door.GetAll()
	local doors = {}

	for _, ent in ents.Iterator() do
		if IsValid(ent) and ent:IsDoor() then
			doors[#doors + 1] = ent
		end
	end

	return doors
end

this gives a list of all doors that have been networked to the client so far instead of just the ones that were networked to the client at round start

however, if you want doors to be always synced and visible to clients, then you would need to set the EFL_IN_SKYBOX flag for every door

i wouldn't recommend this though due to the cost in networking, i guess maybe just for the doorlocker addon and only on doors that need to be highlighted through walls

@TimGoll
Copy link
Member

TimGoll commented Mar 10, 2025

You have a good point here, probably best then to remove the syncing and replace the GetAll function like you suggested. The door locker then can set the flag to all doors when bought(?) or installed. Are the flags persistent over map resets? or do I have to set them again after each reset? If I have to, I'd change it so that the skybox flag is set when the weapon is bought

@wgetJane
Copy link
Contributor Author

You have a good point here, probably best then to remove the syncing and replace the GetAll function like you suggested. The door locker then can set the flag to all doors when bought(?) or installed. Are the flags persistent over map resets? or do I have to set them again after each reset? If I have to, I'd change it so that the skybox flag is set when the weapon is bought

yeah they get reset because map entities will get deleted and recreated on every map cleanup

the flag being set when the weapon is created sounds fine, though on maps with a LOT of doors there may potentially be a noticeable stutter which could clue in the more knowledgeable players that someone has purchased the item

i've also applied the door.GetAll() change to the pr for testing

@wgetJane
Copy link
Contributor Author

btw generally a lot of the buttons in maps not being outlined when you point at them is not due to the button not being solid

it's usually because it's a separate prop_dynamic in the same place as the func_button which is just invisible (using playerclip or trigger texture)

mappers are supposed to parent such prop_dynamic entities to the func_button, however of course not every mapper does that, and there's no way around this unfortunately

@TimGoll
Copy link
Member

TimGoll commented Mar 10, 2025

ohh, that's something I didn't know because I never created a map. I only notices that I was able to fix it by making it solid

@TimGoll
Copy link
Member

TimGoll commented Mar 18, 2025

Image

@wgetJane I'm confused. I changed nothing and the doorlocker is still working. I thought I'd have to set the flag to sync the doors to the client?

@wgetJane
Copy link
Contributor Author

@wgetJane I'm confused. I changed nothing and the doorlocker is still working. I thought I'd have to set the flag to sync the doors to the client?

not sure exactly how the pvs on that map is, it depends on each map

did you move around at all after spawning? you will receive more doors on the client when you move around the map since you will be changing your pvs a lot

@TimGoll
Copy link
Member

TimGoll commented Mar 19, 2025

Not sure either. But this map has area portals in almost all doorframes. So I assumed that this is related. It also looks to me as if I'm able to see all doors from the very beginning on. Did not confirm that though. Is there a map that you know of that is better to test this?

@wgetJane
Copy link
Contributor Author

are you testing in singleplayer or multiplayer

@TimGoll
Copy link
Member

TimGoll commented Mar 19, 2025

multiplayer, TTT2 is broken in singleplayer anyway

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

Successfully merging a pull request may close this issue.

4 participants