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

LZ-624 Alternative Colony Nightmare Insert #3671

Closed
wants to merge 46 commits into from

Conversation

Steelpoint
Copy link
Contributor

@Steelpoint Steelpoint commented Jun 19, 2023

About the pull request

Adds a new map based off of LV-624, at this stage it only changes the main colony.

All equipment spawns, intel and akin are identical to LV-624's original colony.

I'm putting the map up for review to see where it stands.

Explain why it's good for the game

Varity is the spice of life. A new colony to play around in might help invigorate veteran players and those who feel a bit tired of the same map being used so often.

Testing Photographs and Procedure

Screenshot of the SDMM file of the map.

Screenshots & Videos

2023-06-26 04 44 15

Screenshot 2023-06-21 00 53 10
"Casualties of war"

Updated SDMM image as of: 26/06

Changelog

🆑
mapadd: New nightmare colony insert for LV-624 that replaces the colony with an entirely new version.
/:cl:

@github-actions
Copy link
Contributor

You currently have a negative Fix/Feature pull request delta of -2. Maintainers may close this PR at will. Fixing issues or improving the codebase will improve this score.

@github-actions github-actions bot added the Mapping did you remember to save in tgm format? label Jun 19, 2023
@Steelpoint
Copy link
Contributor Author

Not sure what's causing the errors.

@Steelpoint
Copy link
Contributor Author

Maps in working order now and the test errors are fixed.

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

So at this point is there still a need to remove the van? I'm not seeing what error you were experiencing with a van except the one you had already fixed.

code/game/objects/effects/landmarks/landmarks.dm Outdated Show resolved Hide resolved
@Steelpoint
Copy link
Contributor Author

Van is back. Lets see if it works.

@Steelpoint
Copy link
Contributor Author

Looks like the Van still has issues with being deleted.

@Steelpoint
Copy link
Contributor Author

So at this point is there still a need to remove the van? I'm not seeing what error you were experiencing with a van except the one you had already fixed.

I only removed the van as a stop-gap. It seems there's a code issue with the game not properly deleting the van that is being overwritten by the nightmare insert. The rest of the map works from my testing.

@Drulikar
Copy link
Contributor

Looks like the Van still has issues with being deleted.

Try adding SSinterior.interiors -= src to /datum/interior/Destroy()

Copy link
Member

@morrowwolf morrowwolf left a comment

Choose a reason for hiding this comment

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

This is not going to be added as a nightmare insert.

I suggest you look at making this a whole new map (even if LV-624 caves are tagged on the top).

@Steelpoint
Copy link
Contributor Author

Steelpoint commented Jun 26, 2023

@morrowwolf ok.

The issue really is, does the colony not only meet the standards of CM mapping, but is it worth having it as its own map variant?

@Steelpoint
Copy link
Contributor Author

Assuming everything works, I think this is now a standalone map.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

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 Jul 4, 2023
@Steelpoint
Copy link
Contributor Author

The PR gave me some experience but I don't think this is to the standard to become a unique map.

I'll likely revisit to add the hydro+medical as a nightmare insert in the future.

@Steelpoint Steelpoint closed this Jul 11, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jul 15, 2023
#3883)

# About the pull request

This PR applies the fixes previously suggested for #3671. That PR was
originally replacing much of the colony in a nightmare insert and it
exposed how predator landmarks do not handle their area being changed
and an issue with the van being replaced. The description made for these
landmarks is based on some area information and that is used to index
them. This PR makes it so the description of the landmark is saved so it
can still be found in the list even if the area changes.

# Explain why it's good for the game

Less hard deletes and more futureproofing.

# 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: Drathek, Steelpoint
fix: Fixed possible hardeletes for predator landmarks and vehicles.
Predator teleporation descriptions now do not change if the area is
altered at runtime so they can still be found correctly.
/:cl:
@Steelpoint Steelpoint deleted the MapLV624 branch July 17, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mapping did you remember to save in tgm format? Stale beg a maintainer to review your PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants