fluffy-mods / ColonyManager

Colony manager for RimWorld
Other
72 stars 38 forks source link

Feature/update to 1.3 #192

Open skizzix3 opened 3 years ago

skizzix3 commented 3 years ago

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.

skizzix3 commented 3 years ago

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

JordanAdkins commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

JordanAdkins commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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.