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

Make TNT faster by using VoxelManip() for removing nodes. #816

Closed
wants to merge 1 commit into from

Conversation

red-001
Copy link
Contributor

@red-001 red-001 commented Jan 31, 2016

For testing I changed tnt_radius in minetest.conf to 20. Without my patch an explosion took 6 seconds and with it it took one second.

@paramat
Copy link
Contributor

paramat commented Jan 31, 2016

If people complain you might also want to queue 'on destruct' and 'after destruct' functions, although 'on destruct' is usually called before removing a node so this may have problems.

vm:update_liquids()
vm:write_to_map()
vm:update_map()
for i, data in ipairs(on_blast_queue) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is i used ? Please ignore it with an underscore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@red-001
Copy link
Contributor Author

red-001 commented Feb 1, 2016

@kilbith done.

@paramat
Copy link
Contributor

paramat commented Feb 2, 2016

Needs input on whether 'on/after destruct' functions should be queued also.

@red-001
Copy link
Contributor Author

red-001 commented Feb 2, 2016

It should be rather easy for mods to use on_blast instead. It could break some mods that are no longer maintained .

@C1ffisme
Copy link

C1ffisme commented Feb 3, 2016

This should also help with server-freezing chain reactions. 👍

@paramat
Copy link
Contributor

paramat commented Feb 4, 2016

The possibility of destroying locked chests or protected nodes shoud be considered too.

@tenplus1
Copy link
Contributor

tenplus1 commented Feb 5, 2016

Add something like this to check node and if it's a chest drop all items inside.

if nodename == "default:chest"
or nodename == "3dchest:chest"
or nodename == "bones:bones" then

    local meta = minetest.get_meta(p)
    local inv  = meta:get_inventory()

    for i = 1, inv:get_size("main") do

        local obj = minetest.add_item(pos, inv:get_stack("main", i))

        if obj then

            obj:setvelocity({
                x = math.random(-2, 2),
                y = 7,
                z = math.random(-2, 2)
            })
        end
    end
end

@red-001
Copy link
Contributor Author

red-001 commented Feb 5, 2016

@tenplus1 That's really something mods should handle using the on_blast callback. See #806.

@tenplus1
Copy link
Contributor

tenplus1 commented Feb 7, 2016

red-001: the above code was for default chests in game either using tnt mod or edited for on_blast itself... I agree that user mods should handle drops on their own.

@red-001
Copy link
Contributor Author

red-001 commented Feb 8, 2016

@tenplus1 I would agree minetest game should use on_blast more.
But that should be a different PR.

@paramat
Copy link
Contributor

paramat commented Feb 18, 2016

A dev has said there were reasons the voxelmanip wasn't used for removing nodes, you need to research why, try looking through the files history at previous commits. This PR cannot be merged until that research is done.

@red-001
Copy link
Contributor Author

red-001 commented Feb 20, 2016

@paramat I couldn't find any mention of the reasons in the file history or in the comments on #277 but like you said @ShadowNinja said there where reasons for not using voxelmanip. (link to IRC logs) However at the time of the last major rewrite of TNT the on_blast callback wasn't implanted. The on_blast callback has be implanted almost a year ago 8bc8dd6 on_blast can be used similarly to on_destruct for stopping blocks form being destroyed by TNT. An after_blast callback could be added but mods could also remove the node themselves.

@paramat
Copy link
Contributor

paramat commented Feb 21, 2016

There are lots of changes coming for TNT it seems, and too close to release, sofar is busy with his stuff, so it seems the best thing is to delay TNT changes until after release, then do a thorough rewrite.

@sofar
Copy link
Contributor

sofar commented Feb 21, 2016

I'm counting 6 PRs with related changes. I'm rebasing all of them in a single PR, and making it a master WIP TNT PR that can be used to further improve TNT and comprehensively test all the changes collectively, which is rather cumbersome at this point.

@sofar
Copy link
Contributor

sofar commented Feb 21, 2016

Rebased and folded in to #862

@paramat paramat mentioned this pull request Feb 21, 2016
@paramat paramat closed this Feb 21, 2016
@red-001 red-001 deleted the faster_tnt branch March 22, 2016 15:48
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

6 participants