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

Update changeset on Replace checkbox change, other replaced_by fixes #4128

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jul 22, 2024

Problems

After #4127 and KSP-CKAN/CKAN-meta#3277, further testing found more issues with replaced_by:

  • Checking and unchecking the Replace checkbox doesn't update the changeset or enable/disable the apply button
  • If you fix that, changesets with a replace action look weird; they have both a Replace action and a Remove action for the installed mod, and the Install action for the new mod says the reason is "User requested", which is sort of true-ish but really should explain something to do with replacing
  • If you fix that, changesets where one mod replaces multiple installed mods fail on the progress screen

Causes

  • ManageMods.ModGrid_CellValueChanged doesn't react to checkbox changes when the column is ReplaceCol
  • Replace actions were being split into Remove and Install actions without a lot of careful attention to the end result, and SelectionReason.Replacement was not actually in use anywhere
  • The replacement was ending up in the changeset multiple times, with one Install action per replaced mod

Changes

  • Now ManageMods.ModGrid_CellValueChanged updates the changeset when a checkbox changes in the ReplaceCol column, so the apply button becomes enabled when it should
  • Now the changeset will only show Replace actions for the installed mods and an Install action for the replacing mod, with a reason of Replacing <list of mods it replaces>
  • Now each replacement mod gets only one Install entry in the changeset

This should further move replaced_by towards becoming working, useful functionality. It still needs some massaging and refactoring, but now you can actually use it to replace mods.

Note that if you test this with the Nehemiah stuff (somewhat helpful), there are conflicts between some of the old mods and the new one, so you may have to replace them all to be able to continue.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Relationships Issues affecting depends, recommends, etc. labels Jul 22, 2024
@HebaruSan HebaruSan merged commit 63b0518 into KSP-CKAN:master Jul 22, 2024
3 checks passed
@HebaruSan HebaruSan deleted the fix/replace branch July 22, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN GUI Issues affecting the interactive GUI Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant