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

futil and fmods #24

Closed
waxtatect opened this issue Apr 21, 2023 · 8 comments · Fixed by #27
Closed

futil and fmods #24

waxtatect opened this issue Apr 21, 2023 · 8 comments · Fixed by #27

Comments

@waxtatect
Copy link

With last commits, 2 new mods became mandatory dependencies: futil and fmods (https://github.com/fluxionary/minetest-futil, https://github.com/fluxionary/minetest-fmod) and only used through futil with those: futil.check_version and futil.FakeInventory

Would it be possible that futil is optional? and update readme so one can choose, thanks

@wsor4035
Copy link

seems like a wasteful deps that should be removed tbh.

@fluxionary
Copy link
Member

wasteful? the need for using FakeInventory is explained in #23.

@S-S-X
Copy link
Member

S-S-X commented May 13, 2023

Seems like simple solution would be to directly include needed/wanted part (after little cleanup) here or reimplement it here, actual required thing is really simple after all.

@fluxionary
Copy link
Member

Seems like simple solution would be to directly include needed/wanted part (after little cleanup) here or reimplement it here, actual required thing is really simple after all.

and then it has to be maintained in multiple places. i've fixed bugs in FakeInventory before.

@S-S-X
Copy link
Member

S-S-X commented May 13, 2023

and then it has to be maintained in multiple places. i've fixed bugs in FakeInventory before.

Like many things yes, I could also suggest optionally breaking it out from minetest-fmod and providing minetest-fakeinventory mod and having that as dependency but I guess that wont exactly go well with minetest-fmod plans?

edit. or I think it was that other mod, anyway whichever it is. other one seems to provide just version table which seems good as it seems to do one specific thing.

@fluxionary
Copy link
Member

fluxionary commented May 14, 2023

my goal is to make futil an actual library mod - it doesn't do anything except provide useful functions that i regularly use. the exception to this intention is currently in providing a list of items belonging to a group, which could instead be calculated at runtime, if anyone objects. see https://github.com/fluxionary/minetest-futil/blob/7048be0b7b2cf3dded7241ebe0c733dbf8fd1085/minetest/group.lua#L45-L61

i've also switched from using the AGPL to the LGPL for my mods recently, if that's a point of contention, it's no longer relevant.

I could also suggest optionally breaking it out from minetest-fmod and providing minetest-fakeinventory mod

i'd rather not provide a separate mod for every function in futil, but i'm not totally opposed to the idea. perhaps it could instead become a modpack. i'm interested to hear what other people think.

@SwissalpS
Copy link

while I like your efforts with futil, I have to agree that the relative small portion it is used for, is a bit overkill. (see recent pipeworks:autocrafter update that solved the issue with a few lines without the need of loading a full fledged class that copies entire inventory instead of just the list that is actually being processed)

@fluxionary
Copy link
Member

while I like your efforts with futil, I have to agree that the relative small portion it is used for, is a bit overkill. (see recent pipeworks:autocrafter update that solved the issue with a few lines without the need of loading a full fledged class that copies entire inventory instead of just the list that is actually being processed)

if someone wants to write up a solution that gets rid of futil, i won't be offended. but note that the code in question is more complicated than what's currently in pipeworks - it calls minetest.on_craft, which needs an argument that "acts like" a full inventory object. note that pipeworks should be invoking that callback as well, see mt-mods/pipeworks#67.

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

Successfully merging a pull request may close this issue.

6 participants