Skip to content
This repository has been archived by the owner on Dec 29, 2019. It is now read-only.

Fix non-active unknown mods being added, support local database (commented out), stable mod sort within same category, improve ModConfig.xml output handling #25

Merged
merged 9 commits into from Jul 26, 2019

Conversation

lbmaian
Copy link
Contributor

@lbmaian lbmaian commented Jul 21, 2019

Sorry about the multiple changes in a single commit - I was just tinkering with the script for personal use for a while, and later decided to actually create a patch repo.

Changes:

  1. Fix non-active unknown mods being added to ModConfig.xml
  2. Add local database json file option (defaults to "rwms_database.json") that appends/overwrites entries in the remote database json, so unknown mods can be more easily sorted locally
  3. Pretty-print the output ModConfig.xml

@lbmaian lbmaian changed the title Fix non-active unknown mods being added, add local database option, pretty-print XML Fix non-active unknown mods being added, add local database option, stable mod sort within same category, pretty-print XML Jul 21, 2019
@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 21, 2019

More changes:

  1. Address pylint errors and warnings
  2. gitignore the unknown mods files properly
  3. Preserve database ordering of mods within the same category
  4. Allow reordering of mods within the same category via local database

@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 23, 2019

More changes:

  1. Add dump-database option which dumps the combined local and remote database json to stdout
  2. Add dump-database-scores option which dumps the sorted mapping from mod name to sort score as json to stdout
  3. Show relative paths for local unknown mods in the unknown mods report
  4. (semi-regression fix for the new local database functionality) Always include mods in the local database yet not in the remote database in the unknown mods report

@shakeyourbunny
Copy link
Owner

I'm not really a big fan of the option of a local database. Main reason is that I want to prevent splitting up the database to a myriad of clones.

Other changes look good though.

Will look into this the next days or the coming weekend, work load is currently high, but your efforts are really appreciated.

@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 25, 2019

The primary use of the local database is to test additions and changes to mods and their categorization. Currently it's only possible to eyeball whether something looks correct, or else trial and error commits to the database.

There is a risk of people not submitting database issues in lieu of just managing a local database. But overall, I don't think the local database would be used by THAT many people, and each new/updated mod should eventually receive at least one database issue, depending on popularity.

That said, a local database getting stale is a large concern. There should be some mechanism to track whether local entries (or even the local db as a whole) are outdated by recent changes to the remote database. Or at least provide a warning diff of sorts when there are conflicts.

@shakeyourbunny
Copy link
Owner

shakeyourbunny commented Jul 25, 2019

My main concern is primarily that people are investing too much energy in "optimizing" the mod load order when RimWorld itself has not really something defined here. There is one, but this grew chaotically in every other direction in a void without any hard hooks from the game itself, only addon writer defined ones in the style of "there are so many standards I don't agree with, let's make another more perfect one".

"Optimizing" categorization and obsessing over load order will grow into its own pointless minigame when it's mostly good enough already, not perfect mind you, so improvements are always welcome.

Your last paragraph just confirms my concerns, especially you already favour the local dump over the online one and I really do know from experience what's in store in the next steps. Mind you, I have done this local database thingy already myself on another occasion, so please heed my advice.

Please remove anything concerning local databases from the pull request, then I'll happily integrate your modifications.

Copy link
Owner

@shakeyourbunny shakeyourbunny left a comment

Choose a reason for hiding this comment

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

Hi,

please remove anything concerning the local database and renaming the database to the "remote_database". In addition, remove the version jump, this will done by myself.

For the other fixes and additions I am grateful.

@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 25, 2019

I have to disagree that most people would invest too much time into optimizing the load order. Personally, I've only being doing so to get rid of errors. And by errors, either obvious gameplay issues, logged errors, or some other catastrophic failure within Rimworld.

My main problem with only a remote database is the very fact that it takes an indeterminate amount of time to update and without guarantee that every iteration will be error-free enough.

If I'm using a ton of mods, perhaps trying out a couple new ones in the middle of a save, or even just in the testing process of building up a modpack, and some of the mods are unknown or miscategorized, I'd almost always have to do some manual reordering afterwards after automatic sorting to fix errors.

If I didn't have a local database option, I would hardly use this tool. Perhaps once for an initial recommendation, then as I add and change mods around in a modpack, it would be way too cumbersome to rerun the tool and rearrange mods manually afterwards to fix issues... or wait until you update the official database, which depends totally on your own limited time (to point, it's been 8 days since it's been last updated) and hope it doesn't result in any issues every time I do change mods.

The only other alternative I can think of is somehow respect any manual ordering that's been made after an automatic sorting, but I don't know where to even start with that idea.

I can remove the local database options, but I'll always be keeping this fork privately available then for my own personal use. It'd be far too inconvenient otherwise.

In the meantime, I'll remove the version change, and then tinker some more with local database sync safety.

@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 25, 2019

Huh, I was trying to think of a way to toggle off the local database feature via preprocessors, but of course Python isn't a compiled language. But then I just realized that I could just add a constant that toggles it off (including all references in the arguments), requiring anyone that wants a local database to directly modify the script (the closest I can get to requiring recompilation). Would that be a sufficient compromise?

edit: Ended up just centralizing the local db code and commenting that section out.

@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 26, 2019

Alright, I've done the following:

  1. Revert version change
  2. Centralize and comment out all the local database code

Also some more changes:

  1. Always confirm whether to write to ModConfig.xml (there was a case where if there was no unknown mods, the script didn't ask for confirmation)
  2. Backup ModConfig.xml only if a new ModConfig.xml is being written
  3. Always pretty-print written ModConfig.xml (reset-to-core ModConfig.xml wasn't being pretty-printed)

@lbmaian lbmaian changed the title Fix non-active unknown mods being added, add local database option, stable mod sort within same category, pretty-print XML Fix non-active unknown mods being added, support local database (commented out), stable mod sort within same category, improve ModConfig.xml output handling Jul 26, 2019
@shakeyourbunny shakeyourbunny merged commit 4796390 into shakeyourbunny:master Jul 26, 2019
@lbmaian
Copy link
Contributor Author

lbmaian commented Jul 27, 2019

Uh... did you actually try running the script after your own changes? It doesn't work anymore - script outright fails. You reverted too much in load_mod_data if you just wanted to return to intra-category workshop id order. Even if it did somehow run, the unknown mods functionality would be broken again.

I'm also not sure why you want to return to that order either. Sure the intra-category database entry order isn't ideal, but it's still better than the previous version. In fact, os.listdir has no guaranteed order, so it's not even necessarily consistent between script runs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants