-
-
Notifications
You must be signed in to change notification settings - Fork 681
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
Access Mapping Helpers #11469
Access Mapping Helpers #11469
Conversation
Oops, forgot to update the typepath. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing req_one_access is going to be tricky, it works for most accesses but when it comes to maintenance we want people to have both maintenance access and access to a department if they are to be allowed to enter. Maintenance is special in this way, and none of the other accesses really need to work this way. I do like this change though, since its a simplification.
Access on /area I dont think is a good idea, areas are very flaky in the way they are assigned and doors in a boundary may technically belong to both areas. It also modifies access in a way that is hard to tell why, you wouldn't be able to clearly see why a door is suddenly gaining access requirements when in the mapping editor. It also makes it hard to add a door that doesn't have access in an area. Stick to mapping helpers instead of automating it through /area accesses, if /area is going to do access checks then it should be a more refined test that can determine if someone is capable of reaching the area from a public point.
My recommendations:
- Fully remove /area access.
- Find a solution so that doors can require both maintenance access and access to the area that they are connected to. This will probably require something like having a require additional access variable specifically for extra accesses (Since it would be new, I would have it so it has to be applied by the mapping helper and not by var-edits).
@@ -165,6 +165,223 @@ CREATION_TEST_IGNORE_SUBTYPES(/obj/effect/mapping_helpers) | |||
else | |||
airlock.abandoned = TRUE | |||
|
|||
/obj/effect/mapping_helpers/airlock/access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are using airlock mapping helpers, you might as well use its proc /obj/effect/mapping_helpers/airlock/proc/payload(obj/machinery/door/airlock/payload)
that exists specifically to be overriden by child procs to add behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used to, but I could not control it's priority then so they might conflict with req_access_txt or area codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was incorrect in our DMs, access is completely independant of initialisation as its done lazilly in gen_access. You can absolutely move the applicatino to the airlock mapping helper and remove it from the airlock itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to think about a way to be able to lint against airlocks with access var-edits AND access helpers, it's not currently possible with the current maplint though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am currently working on expanding the maplints with 2 new features:
- Required neighbors: If an airlock mapping helper is present, then an airlock must also be present.
- When variable: Rules which only execute if a certain variable edit is present
This does work and has no practical change in-game. I have worked on a larger portion of the map and left roughly 90% of the doors without any set door access with req_access_txt (it being "null"). All doors into the room will require the access of that room. Can't get into Genetics without Genetics access for example. Here is an example. Left door has nothing set and is entirely area controlled. Right one is still under the maintenance (because of emergency maintenance access) and is using a mapping helper to set it to genetics. If we can't people to have both regular department access and access to their nearby maintenance area we simply add it to them. For example Medbay Maintenance could both have Medbay and Maintenance access on them, this can also be handled by area as use support lists.
Indeed they are, that's also why I am not removing req_access_txt, because that will take priority unless we manually remove it. For example from new mapping reworks and so on. So it will be more gradual and controlled. I hope with my emergency access var PR to be able to remove more of the flaky area designation. Perhaps finally doors into maintenance will be powered by the department they are in and not by the maintenance door. No more of these ugly dips.
If in doubt, use the normal method of using req_access_txt, it takes priority over the other two methods. This PR could be merged and neither area code nor mapping helpers could be used and it would not do anything for the most part. Only time it would do something is if you placed a door without any req_access_txt specified in an area with a specified area code. Looking up "access" on an area will show you what access it has.
That has been accounted for. The parent Here is a practical example of a door being inside the surgery area which has the area access code of 45 (surgery), but I want the door into the viewing area to be public. So I've placed down the access marker to nullify the area code.
I don't quite understand what you mean? You wish to have department specific maintenance access codes? Like Medical Maintenance. As a Medbay specific maintenance area. |
To clarify, it was wrong of me to say I've removed it. It has been renamed into "req_access" with the previous implementation of "req_access" being removed. You only need ONE access code to open a door which also has that access code. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
For icon helper, consider implementing this idea: If an airlock is supposed to have some special access format, it should have another appearance tho I mean, if you place two access helpers on a door (2nd and 3rd ones in the image), those will stack over each others, and square bits will be filled to tell accesses (4th one in the image). |
I did try to make a unique icon for each access, but it got quite difficult. I do still have the icons so I could add them in but it feels a bit... irrelevant and easier to simply right click the door to get a list of all the access helpers and their names. One greater issue is standardization. For example Science is entirely FUCKED when it comes to access. I think "tox" with a code of 7 is the default access. While "research" at 47 is R&D. Then "tox_storage" is actual toxins with a code of 8. And on Corg they don't use either, they use Exploration at 49 for toxins!! I'm not going to resolve that here as it feels a bit outside the scope of this PR and it does still work, but it will cause confusion to some in the future. |
I do fucking hate this as well, but the access codes are something legacy that we shouldn't really touch, sadly. |
(This isn't something I'm requiring btw but I'm going to do it anyway in a future PR) |
The lint you need btw should be something like
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request Allows us to test for atoms that are required to be placed alongside another atom, such as APCs to cables. There are many situations where a structure spawner is not good enough: - APC cables need to be in any colour or any direction - Airlock mapping helpers may be any access to any airlock and would create too many subtypes. ## Why It's Good For The Game This allows us to have more comprehensive linting added to future mapping changes. This is needed to support the following lints: - Airlocks should not have an access helper if they have access requirement variables set. - Access requirement helpers MUST have an airlock placed on their tile. Needed to satisfy the following review: BeeStation/BeeStation-Hornet#11469 (comment) ## Testing Photographs and Procedure ### Test required neighbors ![image](https://github.com/user-attachments/assets/ea79aa85-048f-40ca-b27b-2a1da0e1ba2e) ### Conditional Rulesets (I know this rule is useless since it mimics banned_variables, but we can make more complex rules with the other ones and with banned_neighbors) ![image](https://github.com/user-attachments/assets/a607a4cd-0e97-4557-8129-7cf5875a2ebd) ![image](https://github.com/user-attachments/assets/81e43bb6-acc8-441f-97a3-936f5bc88203) Test a more complex query with set operations: ![image](https://github.com/user-attachments/assets/2d309060-8360-4b55-a4d2-2a5d11d7bad5) ![image](https://github.com/user-attachments/assets/33b30d70-3971-40d8-97d5-2f610d73cf86) ![image](https://github.com/user-attachments/assets/50e564eb-9b82-413f-99c3-4655e6c23c75) Super complex query: ![image](https://github.com/user-attachments/assets/0d12c56f-6ff5-4663-bd5b-738c12c3f085) ![image](https://github.com/user-attachments/assets/a46afaa8-4385-4017-a452-a3a624d968c4) ![image](https://github.com/user-attachments/assets/7a6f5fe5-9a0c-4f55-afc9-2fb1777c8a7d) Test like: ![image](https://github.com/user-attachments/assets/2f0187e2-c7f1-4631-8f40-c8d8ae68942b) ![image](https://github.com/user-attachments/assets/1d63faf1-934f-4d6d-8091-f484fcd44c7a) ![image](https://github.com/user-attachments/assets/86dd3593-a3de-47bf-8061-ff206366d387) Not set: ![image](https://github.com/user-attachments/assets/2ae61dfe-4214-42a5-96a9-37ae5e91e6b0) Test is: ![image](https://github.com/user-attachments/assets/ace87258-f9cd-4898-86ba-1cf86bf52a3a) ![image](https://github.com/user-attachments/assets/067acb26-1bf7-4038-9d2f-3e304810fadd) ## Changelog :cl: add: Implements the ability to lint for required neighbors in maplint. add: Adds conditional linting rules in maplint, allowing a lint to apply only if certain conditions are met (Variable is/isn't set, Variable is/isn't a value, Variable matches a regex). /:cl: <!-- Both :cl:'s are required for the changelog to work! You can put your name to the right of the first :cl: if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This PR has been marked as stale due to being in an unmergable state for 7 days. Please resolve any conflicts and add testing evidence, then contact a project maintainer to have the stale label removed. |
About The Pull Request
It is the run of the mill access helper to let you add access to doors. It also changes how req_access works, req_one_access is now a boolean variable. All doors by default use one_access.
I am not a professional programmer, so please be thorough with this PR if you are able to when reviewing.
Why It's Good For The Game
Helps mappers decide access for doors without var editing.
Testing Photographs and Procedure
Screenshots&Videos
Changelog
🆑
add: Added mapping access helpers.
tweak: Made the one-access method the default for any access requirement on all maps.
refactor: Refactored how all codes and one code is handled (all access, one access).
imageadd: Added mapping access helper icons.
code: Changed how objects' req_access and req_one_access is initialized. One access is now handled via a boolean var or mapping helper. Default setting is requiring only one access code.
/:cl: