-
Notifications
You must be signed in to change notification settings - Fork 28
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
Harmony Xeno Beta v1.3 Playtest #27
Harmony Xeno Beta v1.3 Playtest #27
Conversation
Hey looks good. Good work! |
I mostly need to make sure merging this doesn't cause some catastrophically hard to untangle merges in the future. Currently all harmony specific assets are in their respective harmony folders in order to not get clobbered when changes get merged from upstream. So it may take some time for me to go over each file changed and make sure none of it is going to cause the build to explode if something changes upstream. Hope to get it into harmony though! |
Yeah for sure. Looks like this map has a few unique assets so. |
@KeldWolf I'll take a look and see if i can make it fully detached from the upstream code base |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Unfortunately, I had no time to get to this myself. Please move everything you can to Thanks! This is a very cool map. |
Heya, thanks for taking the time to look things over and provide feedback. I can definitely move things around, it shouldn't be an issue. 👍 The map recently went through another playtest and should be updated to reflect feedback first, but once I've done that I will update this PR following the folder recommendation. |
@SlamBamActionman FYI: #92 |
# Conflicts: # Content.IntegrationTests/Tests/PostMapInitTest.cs # Content.Shared/Singularity/Components/ContainmentFieldGeneratorComponent.cs # Resources/Maps/xeno.yml # Resources/Prototypes/Entities/Structures/Shuttles/thrusters.yml # Resources/Prototypes/Entities/Structures/Wallmounts/fire_alarm.yml # Resources/Prototypes/Maps/xeno.yml
This PR should now be ready for review; files have been moved to _Harmony folders where possible, and where not they've been marked via comments. |
Appreciate it! We'll get on reading! |
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.
To sum up.
Important stuff
- 2 linter fails (so far) should be addressed.
Optional based on your best judgment
- Maybe
WallSolidChitin
and its RSIStructures/Walls/xeno.rsi
can be moved. - Maybe
Resources/Maps/xeno.yml
andResources/Maps/Shuttles/cargo_xeno.yml
and references to them can be moved.
I expect this map to end up upstream eventually, since it looks dope and you are a legend.
This should slightly simplify merge and cleanup a bit when that merge comes, but it's not too big of a deal.
Please let me know what you think, and I can't wait to play this!
Thanks for reviewing, I should have moved all requested files and fixed all Linter errors now :) |
I'll ask @KeldWolf to take a final look, and it's good to go! |
@SlamBamActionman might wanna rename the PR to have correct Xeno version in the name. Seems to be 1.3 or 1.3.something. You know better for sure! |
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.
Thanks for you work getting this ready in a way that won't potentially cause us headaches down the road. I'll approve this for trying out on Harmony now.
Great, let me know on Discord whenever a playtest for this station is scheduled, I'll make time whenever! :) |
About the PR
Contains all files necessary to playtest the map Xeno.
File changes:
Enabled
property forContainmentFieldGeneratorComponent
is now a DataField, allowing it to be turned on roundstart. Includes the logic required for it. (Already PR'd upstream)Additions:
Media
Requirements
Breaking changes
Changelog
🆑