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

fix logic of unifieddyes.on_dig to conform w/ documentation #15

Merged
merged 1 commit into from Nov 23, 2023

Conversation

fluxionary
Copy link
Member

currently, there's logic errors in unifieddyes.on_dig. per the documentation,

    on_dig = function(pos, node, digger),
    -- default: minetest.node_dig
    -- By default checks privileges, wears out item (if tool) and removes node.
    -- return true if the node was dug successfully, false otherwise.
    -- Deprecated: returning nil is the same as returning true.

currently, that function sometimes returns nil on failure, sometimes returns nil on success, and sometimes returns the value of minetest.dig_node. this breaks some code i'm writing for my replacer redo, which attempts to not allow the replacer to change the colors of nodes. it also fails to do the other things that minetest.dig_node does, e.g. wear out the tool and call various callbacks.

in this PR, i've copied out the code for minetest.node_dig into a separate function that differs only in that it doesn't ask the lua API to generate the drops, and instead just uses the item without the palette_index metadata.

@OgelGames
Copy link
Contributor

This seems like a lot of extra code to fix a small bug 🤔

@fluxionary
Copy link
Member Author

fluxionary commented Nov 15, 2023

This seems like a lot of extra code to fix a small bug 🤔

if you've got a suggestion for how to get the proper functionality with less code, i'm all ears. i'm somewhat annoyed i had to copy most of that function myself, and upstream could possibly change in the future.

@Niklp09 Niklp09 merged commit 5af9254 into master Nov 23, 2023
2 checks passed
@Niklp09 Niklp09 deleted the fix_on_dig_rvs branch November 23, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants