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

Closets now dense themselves based on pixel offset when being reclosed #5780

Closed
wants to merge 2 commits into from

Conversation

PhantomEpicness
Copy link
Contributor

@PhantomEpicness PhantomEpicness commented Feb 21, 2024

""noooo you can't offset closets into walls and make them non dense on your map"""

ok then how about now

Closets will now check their pixel offset (<10) before making themselves dense again since stuff being offset past a certain point is generally understood to be passable

also updated the vars to 2000s standards

About the pull request

Explain why it's good for the game

makes so mappers have a little less ancient shitcode limiting their mapping

Testing Photographs and Procedure

Screenshots & Videos
2024-02-21.12-10-02.mp4

video featuring a non dense test closet and a normal closet both working as intended.
Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑 Totalepicness5
fix: Closets initially passable will remain passable after closing
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Feb 21, 2024
Copy link
Member

@harryob harryob left a comment

Choose a reason for hiding this comment

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

makes so mappers have a little less ancient shitcode limiting their mapping

initial() doesn't respect changes made in mapping, so this pr does not work

@harryob harryob marked this pull request as draft February 22, 2024 15:52
@PhantomEpicness
Copy link
Contributor Author

makes so mappers have a little less ancient shitcode limiting their mapping

initial() doesn't respect changes made in mapping, so this pr does not work

ok now it checks pixel_x and _y

if those are set it past a certain point communicates to players that it is passable and mappers set these to communicate this too.

@PhantomEpicness PhantomEpicness changed the title Closets will now check if they were dense initially before setting themselves dense Closets now dense themselves based on pixel offset when being reclosed Feb 25, 2024
@PhantomEpicness PhantomEpicness marked this pull request as ready for review February 25, 2024 18:04
@PhantomEpicness
Copy link
Contributor Author

ready for review

will make another test video

@realforest2001
Copy link
Member

There's a var called wall_mounted that does this already, though not always applicable.

Copy link
Contributor

github-actions bot commented Mar 6, 2024

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Mar 6, 2024
Copy link
Member

@SabreML SabreML left a comment

Choose a reason for hiding this comment

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

Forest mentioned the wall_mounted variable, and from some testing that does seem to do the same thing as this PR. The only difference is that the closet would need to have wall_mounted = 1 set in the map editor, rather than it happening automatically.

Is there a reason why that can't be used instead?

Comment on lines +121 to +122
if(pixel_y < 10 || pixel_x < 10) // If it's offsetted to a point where it would confuse players, don't make dense
density = TRUE
Copy link
Member

Choose a reason for hiding this comment

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

Because of the || here, it's setting the closet's density to TRUE if pixel_y OR pixel_x is less than 10, which is the case more often than not (unless it's shifted into a corner or something). You probably want && instead:

Suggested change
if(pixel_y < 10 || pixel_x < 10) // If it's offsetted to a point where it would confuse players, don't make dense
density = TRUE
if(pixel_y < 10 && pixel_x < 10) // If it's offsetted to a point where it would confuse players, don't make dense
density = TRUE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

intended for edge cases, any pixel offset past a certain point communicates to players that it is passable. This wouldn't work if you put it 10 pixels back against a wall and don't adjust X or want to adjust it to be next to something under 10 pixels away for aesthetic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

open to adjusting pixel amounts though as I am unsure of the point where most people consider it "passable"

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the delay! What I meant was that it currently only makes the closet passable if both variables are greater than ten, since that's the 'fail' state of the check:

dreamseeker_UoRn6gMNi8.mp4

(The closet is mapedited to have pixel_y = 25, density = 0)


For comparison of how both versions handle different pixel_x and pixel_y values:

pixel_y < 10 || pixel_x < 10 pixel_y < 10 && pixel_X < 10
Both variables < 10 density = TRUE density = TRUE
One variable >= 10 density = TRUE density = FALSE
Both variables >= 10 density = FALSE density = FALSE

@SabreML SabreML removed the Stale beg a maintainer to review your PR label Mar 6, 2024
@PhantomEpicness
Copy link
Contributor Author

Forest mentioned the wall_mounted variable, and from some testing that does seem to do the same thing as this PR. The only difference is that the closet would need to have wall_mounted = 1 set in the map editor, rather than it happening automatically.

Is there a reason why that can't be used instead?

As I have found out and tested this, it still could be useful as quality of life and reducing instancing.

@Drulikar Drulikar marked this pull request as draft March 11, 2024 20:27
Copy link
Contributor

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Mar 19, 2024
@github-actions github-actions bot closed this Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Fix one bug, make ten more Stale beg a maintainer to review your PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants