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

SinguloStation essentials #2

Merged
merged 31 commits into from
May 16, 2024
Merged

SinguloStation essentials #2

merged 31 commits into from
May 16, 2024

Conversation

Terra1
Copy link

@Terra1 Terra1 commented Mar 4, 2024

About The Pull Request

Adds cryo and the first map, Xyraeon-9b.

Why It's Good For The Game

literally necessary for the server to launch

Changelog

🆑 Terra1, KubeRoot, TheOneAndMightyRed, Urumasi, MaltVinegar, tgstation devs (tralezab), WhiteSands devs (AsciiSquid, MarkSuckerberg, Superyodeler)
add: Cryogenics code.
add: Printable protolathes and techfabs.
add: Xyraeon-9b.
config: Configuration adjusted for SinguloStation.
/:cl:

Copy link

@KubeRoot KubeRoot left a comment

Choose a reason for hiding this comment

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

Changelog is missing authors (AFAIK crediting original code authors in some way is a requirement of the license, not sure why I thought that, seems like it's not based on license, but usually required by listing them out somewhere, but SS13 codebases also accept co-authors on commits, like when cherry-picking)
Changelog is missing changes (no mention of protolathe/techfab/circuit imprinter changes, lacking information on what was actually done to cryo, other changes to things like tracking player login/logout)

Waiting on MDB and IDB to preview new map and check icon changes.
I'd also recommend testmerging this for a test run before it's merged, could get messy due to the lack of atomization.

_maps/map_files/debug/runtimestation.dmm Outdated Show resolved Hide resolved
code/game/turfs/closed/minerals.dm Outdated Show resolved Hide resolved
code/game/turfs/closed/minerals.dm Outdated Show resolved Hide resolved
code/modules/mob/living/carbon/login.dm Outdated Show resolved Hide resolved
code/__DEFINES/dcs/flags.dm Outdated Show resolved Hide resolved
code/modules/mob/living/status_procs.dm Show resolved Hide resolved
code/modules/singulo/ported_procs.dm Outdated Show resolved Hide resolved
code/modules/research/techweb/all_nodes.dm Outdated Show resolved Hide resolved
@Terra1 Terra1 closed this Mar 11, 2024
@Terra1 Terra1 reopened this Mar 11, 2024
icons/obj/cryogenic2.dmi Outdated Show resolved Hide resolved
@Terra1 Terra1 requested a review from KubeRoot March 16, 2024 19:27
@Terra1 Terra1 requested a review from KubeRoot May 12, 2024 16:10
Copy link

@KubeRoot KubeRoot left a comment

Choose a reason for hiding this comment

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

Good job fixing the issues, there's just one thing I'd like corrected before I approve this.

code/modules/mob/living/carbon/logout.dm Outdated Show resolved Hide resolved
Copy link

@KubeRoot KubeRoot left a comment

Choose a reason for hiding this comment

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

Approved, noting my recommendations that have been dismissed:

  • I think we should avoid modifying files where not necessary, in this case runtimestation - resolving conflicts on that would get very annoying.
  • I think you shouldn't rename the locate_weakpoint file and instead comment out the entire thing, once again for conflict reasons.

I will also note there's still multiple places where singulo-specific code is not marked, but that doesn't affect how the code runs, and my understanding is that you want to handle merges yourself, so that's up to you.

The changelog was also never completed despite my suggestion, but as long as attribution information is correct, I don't really care.

@@ -176,10 +176,10 @@ GUEST_BAN
# WIKIURL http://www.tgstation13.org/wiki

## Rules address
# RULESURL http://www.tgstation13.org/wiki/Rules
RULESURL https://discord.gg/yjCSCzzezp

Choose a reason for hiding this comment

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

Putting your rules on discord seems like a bad idea and dick move for anybody who doesn't want to use discord, but config is your territory.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that needs to be changed in the future; we're moving the rules to the wiki anyway.

@Terra1 Terra1 merged commit 736523b into master May 16, 2024
20 checks passed
github-actions bot added a commit that referenced this pull request May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants