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

No burrow surfacing on lifeboats/unburrowable areas #5603

Closed
wants to merge 40 commits into from
Closed

No burrow surfacing on lifeboats/unburrowable areas #5603

wants to merge 40 commits into from

Conversation

hislittlecuzingames
Copy link
Contributor

@hislittlecuzingames hislittlecuzingames commented Jan 31, 2024

About the pull request

This is to prevent burrowers from surfacing in areas marked as not tunnleable.
IE the lifeboats in an evac of the almeyer.

Explain why it's good for the game

I think surfacing was an oversight of the original burrow ability.
This will stop goofy burrows that are nonsensical like a burrower emerging inside a lifeboat during a hijack.
HOWEVER, it doesn't stop burrowers from burrowing on the almeyer. Only /area/foobar that are explicitly tagged as flags_area = AREA_NOTUNNEL cannot be emerged from now. before it was just preventing burrowing down into these tiles.

Testing Photographs and Procedure

Map changes were not included. Only code changes were made. Chance's claim is just my default map for some reason, so I used its areas and edited them temporarily for testing.

https://youtu.be/EwlO7mJ017s

Changelog

🆑
fix: fixed burrower ability. Disallowed ability to surface in areas marked as not tunnel-able.
/:cl:

@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Jan 31, 2024
@cm13-github
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added Mapping did you remember to save in tgm format? Fix Fix one bug, make ten more labels Jan 31, 2024
@hislittlecuzingames
Copy link
Contributor Author

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Yeah I gotta fix my base branch-has some artifacts.

SHould be good now.

@realforest2001 realforest2001 marked this pull request as draft January 31, 2024 12:36
@hislittlecuzingames hislittlecuzingames marked this pull request as ready for review January 31, 2024 18:02
@Birdtalon Birdtalon added the Balance You need to be a professional veteran game maintainer to comprehend what is being done here. label Jan 31, 2024
@Nanu308 Nanu308 removed the Mapping did you remember to save in tgm format? label Feb 2, 2024
@hislittlecuzingames
Copy link
Contributor Author

Most of the changes were to fix the small grammer stuff.

The fix is very small.

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 Feb 10, 2024
Comment on lines +101 to +105
var/area/current_area = get_area(T)
if(current_area.flags_area & AREA_NOTUNNEL)
to_chat(src, SPAN_XENOWARNING("There's no way to surface here!!"))
return

Copy link
Contributor

Choose a reason for hiding this comment

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

This is unnecessary; its already done further down at line 129/134 so should revert this change.

The difference is you spend some time trying to tunnel there and find out you can't - which is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without that code, it wouldn't work.
It lets them surface.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you test this? Because I can comment it out and it still is checked because its a duplicate check as I said.

So either you need to more clearly specify what conditions require this duplicate check; but I don't currently see any situation this isn't simply redundant.

image

code/game/area/shuttles.dm Outdated Show resolved Hide resolved
@Drulikar Drulikar added Balance Approved This PR has had its balance and gameplay-affecting aspects approved. Cry to the Head-maint about it. and removed Stale beg a maintainer to review your PR labels Feb 12, 2024
@Drulikar Drulikar marked this pull request as draft February 12, 2024 01:03
@hislittlecuzingames
Copy link
Contributor Author

Going to re-test it another day since I committed suggested changes.

Also going to verify my comment that I needed the code stated to be unnecessary.

Comment on lines +101 to +105
var/area/current_area = get_area(T)
if(current_area.flags_area & AREA_NOTUNNEL)
to_chat(src, SPAN_XENOWARNING("There's no way to surface here!!"))
return

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you test this? Because I can comment it out and it still is checked because its a duplicate check as I said.

So either you need to more clearly specify what conditions require this duplicate check; but I don't currently see any situation this isn't simply redundant.

image

@hislittlecuzingames
Copy link
Contributor Author

I said I would test at a later date.
#5603 (comment)

@hislittlecuzingames hislittlecuzingames closed this by deleting the head repository Feb 21, 2024
github-merge-queue bot pushed a commit that referenced this pull request Feb 22, 2024
# About the pull request

This PR effectively revives the fix from #5603 that was allowing
burrowing in lifeboats. The other grammar changes it was doing were for
marines, so it would not be correct to we/our them.

# Explain why it's good for the game

Fixes burrowing into lifeboats.

# Testing Photographs and Procedure
<details>
<summary>Screenshots & Videos</summary>

Put screenshots and videos here with an empty line between the
screenshots and the `<details>` tags.

</details>


# Changelog
:cl: AtticusRezzer
fix: Fixed the incorrect area flag in lifeboats allowing tunneling
/:cl:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Balance Approved This PR has had its balance and gameplay-affecting aspects approved. Cry to the Head-maint about it. Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Fix Fix one bug, make ten more
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants