-
Notifications
You must be signed in to change notification settings - Fork 7
Add command for removing a mod from a pack #37
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
base: main
Are you sure you want to change the base?
Conversation
|
Ok, so looking at commit 1f8d0ef, it seems level is the dependency type, I had interpreted it as a sort of hierarchy depth. I thought I was filtering to 1, but I just realized that was only in my test query. I can see that 1 == Required, I assume 2 == Optional, what is 3? Thanks. |
|
Hmm, maybe I missed it, but I'm not seeing any code that ensures you're not inadvertently removing a dependency that another mod requires? I.e. If mod X and Y depend on mod A, and the user removes mod X, mod A will also get removed, thus breaking Y. |
|
Hold off on reviewing this too deeply for now since I'm overhauling now that I know what I actually spent a good deal of time yesterday reworking the recursive code since I figured out what Also, FYI, while working yesterday, I noticed that the new CREATE TABLE deps_dg_tmp
(
fileid INTEGER,
projectid INTEGER,
level INTEGER,
CONSTRAINT deps_pk PRIMARY KEY (fileid, projectid)
);
INSERT INTO deps_dg_tmp(fileid, projectid, level) SELECT DISTINCT fileid,projectid,level FROM deps;
DROP TABLE deps;
ALTER TABLE deps_dg_tmp RENAME TO deps;
CREATE INDEX deps_fileid_index ON deps (fileid);
CREATE INDEX deps_projectid_index ON deps (projectid); |
|
Thanks for pointing this out about the deps table. I'll tweak the crawler to use a primary key on deps and rebuild everything. |
|
@mgziminsky - I've updated the crawler to have a primary key on deps table; let me know if that addresses your performance issues, please? The latest db available on S3 should reflect this change. |
|
Thanks, that did the trick. I'm in the process of putting the finishing touches on my code and should have this updated soon. Edit: Removed missing db entries remark and log output to remove clutter from the PR. Issue was resolved via discussion in discord. |
|
@mgziminsky - could you ping me on the Forge discord or some other system with IM? I'm dizzyd#7825 on Discord or @dizzyd on Twitter. |
b2b9f79 to
fe30007
Compare
There are 2 commands: single and recursive. The single version only removes the 1 mod that was specified, ignoring dependencies. The recursive version will remove the requested mod and all dependents and dependencies.
Also, don't remove dependencies that are optionally required by any remaining mods
04d595a to
2f7ed12
Compare
For this, I implemented both a basic, single mod version, and a recursive version that also removes any mods that depend on the one being removed. The recursive version is "disabled" though, since I couldn't find any way to distinguish between required and optional dependencies.
I'm not sure how you generate your database, but would it be possible to add that information into the moddeps table? If it's possible, I'd be willing to do it if you directed me to the code.