iquercorb / OpenModMan

Open Mod Manager - Open source and generic Mod ("Modifications") manager.
GNU General Public License v3.0
88 stars 5 forks source link

[BUG/Wrong Logic] Deletion already installed mods #94

Closed lmiol closed 2 months ago

lmiol commented 2 months ago
  1. Install mod
  2. Move the mod to the recycle bin (as a result, mod disappears from the menu)
  3. Restart the app (as a result, we can see the uninstalled mod with 2 icons concurrently)

I do not know what logic is expected, but if we try to double-click on that mod, the mod be uninstaled, but if try more double click it will be installed from next line (in my case, Kings Pass - Parallel Stage).

here is a video https://github.com/sedenion/OpenModMan/assets/264879/f8e9961b-a2ea-4a34-ac02-16a2053587de

It looks like a bug (install from second line) and not complete logic (why mod dissapears and appears after). I think there should be some other logic, here is a few solutions

  1. If we try to delete an already installed mod, there should be an alert: "This mod is already installed. What should we do with the installed files?" Delete, Keep, Cancel Delete: deletes the mod and installation files Keep: deletes the mod, but the files stay, and after a restart, we would not see this mod in the menu Cancel: cancels the deletion

  2. The current deletion alert should contain: "Move the selected mods to the recycle bin. Installed files will also be removed." That means all installed files will be removed also. And after a restart, we would not see this mod in the menu

  3. There is also a scenario in which after deleting a mod into the trash, the mod does not disappear and still appears immediately (without restarting), but it needs to be fixed so that double-clicking does not install the next mod and does not work at all (since the mod is already deleted and these are just files), and only the Uninstall should work which in the end should essentially lead to the deletion of files. However, Move to recycle bin should not be shown here already. Because if you do it again, it leads to the deletion of another mod and not the current one.

In general, I consider point 3 to be an improvement of the current logic, but in my opinion, it is the most unfamiliar and incomprehensible. Usually, when deleting something, you are offered to delete the remaining remnants or they are deleted automatically. Therefore, I suggest option 1 or 2.

reproduced 1.2.7 (1.2.8 too)

Regards

iquercorb commented 2 months ago

Indeed this is a bug, installed mod should not disappear from list when deleted, but being displayed with red cross (like after you restarted), then when Mod is uninstalled, be effectively removed from list. Fixed in source code.

lmiol commented 2 months ago

Indeed this is a bug, installed mod should not disappear from list when deleted, but being displayed with red cross (like after you restarted), then when Mod is uninstalled, be effectively removed from list. Fixed in source code.

I hope you choose 1 or 2. Because there more bugs would be with the red cross logic. I will retest your fix in new release. But my error guessing tells me bugs would continue

Also if we delete mod why I want to keep installed files? It is a small chance to want it., not a usual case. Usual case is delete mod with files. So you force users always to do 2 actions instead of 1

iquercorb commented 2 months ago

Once deleted while installed, I let the Mod Pack visible with a red cross (indicating the source is not present, while backup data still here) with ability to be uninstalled. This because uninstall is a complexe process that run within a dedicated thread, can throw error, etc. and so would require special handling to be included within batch deletion (mixing deletion error handling and restoration error handling)... Separating deletion task from uninstall task appear less logical to user, but it is safe and way more easy to implement correctly.

lmiol commented 2 months ago

Once deleted while installed, I let the Mod Pack visible with a red cross (indicating the source is not present, while backup data still here) with ability to be uninstalled. This because uninstall is a complexe process that run within a dedicated thread, can throw error, etc. and so would require special handling to be included within batch deletion (mixing deletion error handling and restoration error handling)... Separating deletion task from uninstall task appear less logical to user, but it is safe and way more easy to implement correctly.

I told you how to do best user experience of usual interface of same logic. That's all