Skip to content

Feature/update to 1.3 #192

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

Open
wants to merge 5 commits into
base: 1.2
Choose a base branch
from

Conversation

skizzix3
Copy link
Contributor

I gave my best with trying to update it.
Tested the mod and did not experience errors at the end.
My knowledge isn't the greatest, when it comes to Rimworld modding.

Please double-check the QuickSearch-Filter in "UIThingFilterSearchable.cs", because it kinda feels like a dirty fix.
Not sure though.

Hopefully I was able to help.

Copy link

@JordanAdkins JordanAdkins left a comment

Choose a reason for hiding this comment

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

I have downloaded the branch and ran some tests and this does not seem to work for me.

I cannot test the actual behavior, as the UI itself is broken and does not allow me any sort of interaction other then switching tabs.

Tested on the latest stable branch of Rimworld 1.3 as of 7/23/2021 with the following installed loaded in the listed order:
Harmony
Rimworld Core
Rimworld Royalty
Rimworld Ideology

Tested in both full screen and windowed mode at a variety of resolutions.

@skizzix3
Copy link
Contributor Author

skizzix3 commented Jul 24, 2021

I forgot to add the assembly to the request after rebuild, my bad.
Pushed it now.

@JordanAdkins
Copy link

My apologies for not rebuilding myself when I tested it. I have never worked with C# and wasn't aware it was something I would need to do. I also meant to include the stack trace, sorry if that made it any harder to figure out.

I have tested all the functionality and have ran into no issues so far. 👍 Until fluffy is able to review these changes or update it themselves I will be using this build. I will report back here if I encounter anything.

Thank you for your work skizzix.

Tested on the latest stable branch of Rimworld 1.3 as of 7/24/2021 with the following installed loaded in the listed order:
Harmony
Rimworld Core
Rimworld Royalty
Rimworld Ideology
Colony Manager

One note though, not sure how Fluffy normally manages their branches, but I am not sure if PRing this into the 1.2 release branch is exactly correct. Perhaps it needs repointed toward master?

@skizzix3 skizzix3 marked this pull request as ready for review July 24, 2021 18:37
@skizzix3 skizzix3 changed the base branch from 1.2 to master July 24, 2021 18:37
@skizzix3 skizzix3 changed the base branch from master to 1.2 July 24, 2021 18:38
@skizzix3
Copy link
Contributor Author

Hope you did not got spammed with notifications too much, but when i change the base to master, it changes from 7 files changed to 84 files changed. Not sure if master is outdated or 1.2 is behind master. Did not think about checking before switching the base back.

I'll just wait for Fluffy to look at this.

Thanks for the review & testing! :)

@SaberVS7
Copy link

SaberVS7 commented Jul 24, 2021

Don't forget to update the About XML to point to 1.3 - Noticed it was still set to 1.2 when I went to change the PackageID in my local copy to something different than the Workshop build so I wouldn't have to unsubscribe.

@jacobgc
Copy link

jacobgc commented Jul 25, 2021

IMO this should probably go into a new 1.3 branch right? master is currently at a point before 1.2.

@JordanAdkins
Copy link

JordanAdkins commented Jul 25, 2021

@jacobgc I agree, but unless there is some trick that I don't know of, it is only possible to point a PR to an existing branch. Fluffy would have to create a 1.3 branch and then have this PR repoint towards it.
I made the previous comment that I thought it should be pointed toward master with the assumption that master would then be branched off into a 1.3 release once everything has been 100% verified. However, it seems that master is quite different then 1.2 so I am not sure how its being managed.

@jacobgc
Copy link

jacobgc commented Jul 25, 2021

@JordanAdkins Yeah, I think only Fluffy can create the target branch. From what I can tell, it seems master is targeted towards the last beta release of rimworld, and then for every major rimworld version a new branch has been made (1.1,1.2 etc)

@FluffierThanThou
Copy link
Member

my apologies for the slow reply! I'm going to be integrating this into the 1.3 update now.

Have you guys noticed any weirdness with this and the new auto-slaughter thing interacting?

As for what branch to target, 1.2 is fine, and the most recent branch. I create a new branch for each RW update, and set that as the default branch when the release becomes public. Obviously, I was a bit slow for that this time around ;).

@FluffierThanThou
Copy link
Member

rebased and merged onto the new 1.3 branch on my end - if you want to make further changes please do so from the 1.3 branch.

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.

5 participants