minetest-mods / smart_inventory

Minetest inventory with focus on very much items
GNU Lesser General Public License v3.0
8 stars 5 forks source link

Using the smart_inventory_swapline_button in nodecore discards all items. #14

Open mk-pmb opened 3 years ago

mk-pmb commented 3 years ago

I expected it to fail, but not this destructively.

bell07 commented 3 years ago

I do not know nodecore. Short look to the mods list say me it's a game not based on minetest_game, and does not use the "creative" mod. Therefore unsure if the nodecore can be fully supported with smart_inventory

0-afflatus commented 3 years ago

Nodecore is one of the most original new games to use the minetest engine. I doubt whether it contains any minetest_game code at all. I have only attempted to play it a couple of times, but If I understand the game concept correctly, creative mode would be anathema to the game dynamics. Nodecore has its own unique mods, I would expect all mods designed for MTG to break.

mk-pmb commented 3 years ago

Looks like a pebkac error on first inspection.

I hope that's just a misunderstanding, not an insult. Compare how Smart Inventory looks in Voxelgarden and in Nodecore. As you can see, the mod is able to detect that there are less item slot rows available, at least it doesn't paint them. So the least it could do is hide the missing rows' swap buttons, or show a warning that "swap" will effectively be "discard" in this case.

0-afflatus commented 3 years ago

It's an indication that a better explanation of the problem is required. Nodecore doesn't use an inventory like MTG based games do, it only uses a single row. I didn't realise that it does support creative mode, but surely any mod you use with NC needs to be modified specifically to work with that game. You haven't explained why you expect this mod to work with NC, most of the mods in this repository can't be expected to. Unless a nodecore developer states that it ought to work or someone comes up with a patch, I think this is a non-issue.

0-afflatus commented 3 years ago

I apologise for the pebcak comment. I expect to have to customise mods for my own games, which don't deviate from MTG as much as NC. It's nice when you don't have to, but the solution here is to create a nodecore compatible version of smart inventory surely?

mk-pmb commented 3 years ago

You haven't explained why you expect this mod to work with NC,

In this case I was just optimistic. I like how it worked in Voxelgarden, so I tried it in Nodecore as well, and there were no obvious signs of any incomatibility, so I assumed it would work. And for the most part, it does work. :-) At the time I didn't know what those symbols meant, so I thought I'd just try. I had expected that they would either do nothing (because there was no item row besides them), or that it would create such an item row (probably in its own memory). I would not have imagined that it discards all my items with no warning.

but the solution here is to create a nodecore compatible version of smart inventory surely?

That would be the nicest. However, in the short term, it would be ok to just hide or disable the swap buttons if they have no target.

bell07 commented 3 years ago

There is no way to hide or disable the buttons. But you can remove the implementation part. Look into pages/creative.lua Remove the lines 211-223

    -- swap slots buttons
    state:image_button(0, 6, 1, 1, "swap1", "", "smart_inventory_swapline_button.png"):onClick(function(self, state, player)
        ui_tools.image_button_feedback(player, "creative", "swap1")
        state.param.invobj:swap_row_to_top(2)
    end)
    state:image_button(0, 7, 1, 1, "swap2", "", "smart_inventory_swapline_button.png"):onClick(function(self, state, player)
        ui_tools.image_button_feedback(player, "creative", "swap2")
        state.param.invobj:swap_row_to_top(3)
    end)
    state:image_button(0, 8, 1, 1, "swap3", "", "smart_inventory_swapline_button.png"):onClick(function(self, state, player)
        ui_tools.image_button_feedback(player, "creative", "swap3")
        state.param.invobj:swap_row_to_top(4)
    end)
mk-pmb commented 3 years ago

There is no way to hide or disable the buttons.

In that case, their handler function swap_row_to_top should do nothing if the target row doesn't exist.