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

NPC's no longer 'stick' on some doors #46505

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

Emotions211
Copy link
Contributor

@Emotions211 Emotions211 commented Jan 3, 2021

Summary

SUMMARY: Bugfixes "Fixes doors that NPC's could not pass without intervention."

Purpose of change

Resolves #46124.
Resolves #44294.
Resolves #37207.

Changes issues with #46472.

NPC's getting stuck on various doors and gates can be annoying, at best, suicidal for them at worst. In some cases like the Freshwater Research Station. They're completely unable to traverse it on their own and either need the doors to be removed, or for you to swap positions with one while standing on the door to get them through.

Describe the solution

Removed the "DOOR" flag from several doors/gates. NPCs seem to get stuck attempting to open already open doors that have this flag. Removing it fixes it.

Describe alternatives you've considered

Not fixing the bug.

Testing

Added the "DOOR" flag to several open doors and attempted to bring NPC's through. They got stuck. Removed it and they're able to pass through just fine. Did so with doors in question and they can now pass through without issue.

Additional context

During review, the "DOOR" flag should be removed from any open variants. It isn't needed for an NPC to recognize the terrain as a door that can be closed.

Removed "DOOR" flag from several open doors that prevented NPC's from moving through them on their own.
@Theawesomeboophis
Copy link
Contributor

Can player enter doors still as well ( According to the .md, the DOOR flag is what makes them openable as far as I understand)? Also remove the [ ] around bugfixes, that's what's causing the PR validator fail.
https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/JSON_FLAGS.md#flags-2

@Emotions211
Copy link
Contributor Author

Yes the player can still pass through them. The DOOR flag was on several open variants and NPC's were unable to pass through them. I believe they check if the door flag is present then attempt to open it. As such they were stuck in a loop.

It's only been removed from those open variants. NPC's and the player can still close them.

@anothersimulacrum anothersimulacrum added the <Bugfix> This is a fix for a bug (or closes open issue) label Jan 3, 2021
@Theawesomeboophis
Copy link
Contributor

Yes the player can still pass through them. The DOOR flag was on several open variants and NPC's were unable to pass through them. I believe they check if the door flag is present then attempt to open it. As such they were stuck in a loop.

It's only been removed from those open variants. NPC's and the player can still close them.

Oh so It's only open doors that were already open and NPCs were getting confused with?

@Emotions211
Copy link
Contributor Author

Yeah, the oldest issue mentioned is almost a year old now and had to do with "t_screen_door_o". A later one was "t_gate_metal_o" and I found the issue with "t_door_metal_bulkhead_o". This whole issue seems to have started 15 months ago with the addition of the screen doors and other furniture with this pull #34734. Others came later and likely copied from there for some reason.

@Theawesomeboophis
Copy link
Contributor

Theawesomeboophis commented Jan 3, 2021

Yeah, the oldest issue mentioned is almost a year old now and had to do with "t_screen_door_o". A later one was "t_gate_metal_o" and I found the issue with "t_door_metal_bulkhead_o". This whole issue seems to have started 15 months ago with the addition of the screen doors and other furniture with this pull #34734. Others came later and likely copied from there for some reason.

Nevermind issue seems to be that npcs are trying to Open open doors and can't because there already open, causing the npc to get confused and not be able to enter the door.

@Emotions211
Copy link
Contributor Author

Emotions211 commented Jan 3, 2021

They could OPEN the closed door (t_screen_door_c changes to t_screen_door_o) but then they would continue to attempt opening the now opened door. They can open doors just fine, but any open (tile id ends in an o) door with the "DOOR" flag, they would treat as closed.

The only way to move an NPC through them is to stand ON the tile, swap positions with them, then push from the tile they were just on. For some reason there isn't a check to see if an NPC can stand where you're currently standing when you swap with them.

When you open a door, it's actually completely changing the tile to a different one. The opened one does not need a flag to declare it as a door

Edit: nevermind you got it

@Theawesomeboophis
Copy link
Contributor

Is this a problem with any other open variants of doors or just these specific doors?

@Emotions211
Copy link
Contributor Author

Emotions211 commented Jan 3, 2021

Just these 3 doors. I went through both .json files and only found these 3 with the out of place "DOOR" flag.

In the future, when adding doors, the open variants shouldn't have the "DOOR" flag since it's only needed for a NPC to know to open it. As far as I can tell, the player can still interact with doors without this flag.

Heres the full quote for the flag in the MD

"DOOR Can be opened (used for NPC path-finding)."

Keyword is opened. These doors are already open and shouldn't have this flag.

Also the fact it says "(used for NPC path-finding)" implys it isn't needed for a player to open.

@Emotions211
Copy link
Contributor Author

Emotions211 commented Jan 3, 2021

Looks like Magiclysm caused the travis fail? What?

No clue what caused the travis fail like everytime I attempt a PR in a project. Any clue what caused it anyone?

@BrettDong BrettDong added [JSON] Changes (can be) made in JSON Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. NPC / Factions NPCs, AI, Speech, Factions, Ownership labels Jan 4, 2021
Copy link
Member

@BrettDong BrettDong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and can confirm NPC now passes through:

螢幕截圖 2021-01-04 上午12 30 33

(patched)

NPCs are trapped in master version:
螢幕截圖 2021-01-04 上午12 30 01
(master)

@ZhilkinSerg
Copy link
Contributor

This could possibly have some unintended effects - DOOR flag is used for some other checks in tile rendering, mapgen, start location code.

@BrettDong
Copy link
Member

I looked at other doors and most opened variants do not have DOOR flag.

@ZhilkinSerg ZhilkinSerg merged commit 3987c51 into CleverRaven:master Jan 4, 2021
@Emotions211
Copy link
Contributor Author

Emotions211 commented Jan 4, 2021

I looked at other doors and most opened variants do not have DOOR flag.

This is what lead me to test it. The documentation says it's used for NPC Pathing and that's it. In testing they were still able to close them as well if told to.

JSON_FLAGS.md says this for the DOOR flag. It isn't needed on closed doors and just breaks them for NPCs.

DOOR Can be opened (used for NPC path-finding).

@Zireael07
Copy link
Contributor

Looks like JSON_FLAGS.md needs to explicitly say what the flag is used for and when NOT to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. [JSON] Changes (can be) made in JSON NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
6 participants