Disabling TNT #1085

tenplus1 · 2016-05-09T19:57:31Z

Disabling TNT shouldn't remove it altogether, removing the TNT craft recipe and the tnt:tnt node is enough, leaving tnt:tnt_burning available for 3rd party mods like lucky blocks to use for explosions.

sofar · 2016-05-09T20:27:57Z

OK, this isn't so bad. I wouldn't mind this getting merged.

C1ffisme · 2016-05-09T21:12:32Z

Or... What about allowing the recipe, but TNT cannot be lit by people without a privilege? (Maybe protection_bypass? IDK)

This means TNT can still be used for decorative purposes. (Although you run the risk of stupid people becoming moderators.)

Just throwing this idea out there.

sofar · 2016-05-09T21:17:08Z

@C1ffisme there's a background to this PR. @tenplus1 is attempting to make the TNT API usable for mods on multiplayer servers. Hence adding a priv isn't what he wants or needs.

C1ffisme · 2016-05-09T21:34:26Z

This would be usable for mods, actually. A priv would have to be given, but unless you have twenty moderators, it wouldn't be that hard to change. (Remember when we added noclip? You probably don't, but it was annoying.)

sofar · 2016-05-09T23:28:03Z

Mods have little use for privs. Privs are for users.

@tenplus1 just wants his cows to explode using the TNT api, without having TNT blocks available to players.

C1ffisme · 2016-05-10T16:09:34Z

Oops, I'm pretty sure we mean entirely different things.

I meant 'mods' as a shorter term for 'moderators'.
You meant 'mods' as a shorter (and more used) term for 'modifications'.

My idea was that users can still place TNT, but cannot light it. However, a user with a certain priv, such as 'tnt' or 'protection_bypass' could light TNT. (I thought of protection_bypass because you can blow up TNT right next to a protected area, and destroy blocks in that area.)

numberZero · 2016-05-11T15:50:21Z

a user with a certain priv ... could light TNT

This way gunpowder needed to be disabled too, as well as mesecons. In my opinion, the best way is to disable TNT igniting entirely, but keeping tnt:tnt for decorative purposes (HE warehouse?) and maybe keeping the tnt:tnt_burning node too for mods.

If you really think moderators need TNT and not WorldEdit, let them have “admin_tnt” that is not craftable at least, and maybe can only be ignited directly by a user with certain privileges

sofar · 2016-05-11T16:42:26Z

This is starting to go entirely off-topic.

the patch by @tenplus1 is fine, as-is (maybe also remove gunpowder node?). If you want additional features please request them in a separate issue.

I don't want a cosmetic TNT block. minetest_game doesn't do stuff like that - there is no cosmetic chest, there is no cosmetic furnace, no cosmetic apple, etc..

stujones11 · 2016-05-11T18:26:31Z

+1 On-topic, this would also be very useful for my shooter mod. I would prefer to keep the gunpowder node if possible, although it would probably be more consistent to remove that too.

C1ffisme · 2016-05-11T21:54:37Z

@sofar Yes, this is going off-topic...

(Actually, there are cosmetic glasses and bottles, and cosmetic shelves...)

sofar · 2016-05-11T22:08:14Z

They are not intended to be cosmetic, and the shelves are functional? I believe you can even put water in the bottles?

DonBatman · 2016-05-11T22:25:52Z

I think this pr should be left as is and let mods add decorative tnt. I think majority of servers that disable tnt would not care for deco tnt imho.

C1ffisme · 2016-05-12T00:34:15Z

@sofar Nope. The bottles cannot be filled with water. Even if they could, you couldn't do much with them. (Implement a brewing system?)

The bookshelves can store your books, and with written books, it even has a purpose. But vessels shelves can store items (bottles) that you don't need in the first place. (Unless they get a use at some point.)

Actually, the original purpose of the bookshelves, before they had inventories, was to be a decorative node, and solely a decorative node. An expensive one back then too, since I'm not sure papyrus could be farmed.

@DonBatman Actually, that sounds like a better idea. I wanted to throw out this idea for anyone to hear, but it just turned into an argument...

red-001 · 2016-05-20T20:03:28Z

Wouldn't it be better to not disable tnt.register but just not register tnt:tnt?

sofar · 2016-05-20T20:15:56Z

sure, yes, that would be better.

paramat · 2016-06-26T22:32:27Z

mods/tnt/init.lua

@@ -4,7 +4,7 @@ local singleplayer = minetest.is_singleplayer()
 local setting = minetest.setting_getbool("enable_tnt")


'setting' needs a more descriptive name like 'enable_tnt'

paramat · 2016-06-26T22:46:00Z

I support this PR.

l just noticed that settingtypes.txt has to be in minetest_game/ and not in minetest_game/mods/tnt/

This needs fixing.

And bool obviously can be set only to true or false but not to nil.

'minetest.setting_get' can return nil, but see my line comments the 'local setting' should be true or false.

If you add another commit i can squash these on merge.
Note: the 3rd commit should be discarded on squash as it is a duplicate.

paramat · 2016-06-26T22:53:48Z

Please also disable the ABM.

paramat · 2016-06-27T21:11:07Z

mods/tnt/init.lua

-		(singleplayer and setting == false) then
-	return
+local enable_tnt = minetest.setting_getbool("enable_tnt")
+if (not singleplayer and not enable_tnt) or


getbool can return 'nil' so this line should not be changed, as it was checking for setting == false or setting == nil

paramat · 2016-06-27T21:13:24Z

Please only enable the ABM if 'enable_tnt'.

t4im · 2016-06-28T15:10:48Z

mods/tnt/init.lua

+local enable_tnt = minetest.setting_getbool("enable_tnt")
+if (not singleplayer and enable_tnt ~= true) or
+		(singleplayer and enable_tnt == false) then
+	enable_tnt = false


The entire if-statement can be removed. It doesn't serve a purpose without the return anymore.

The second part of the statement sets enable_tnt only to false if it already is false.
The first part when it is either false or nil in multiplayer (which doesn't make a difference to the enabled state later, because false and nil are treated there the same)

tenplus1 · 2016-06-29T06:29:01Z

Having server admin properly set TNT status is a good thing, same with disabling fire. Minetest should only set a default and not choose depending on singleplayer/creative status.

paramat · 2016-06-30T19:07:42Z

Defaulting to false on multiplayer is for good reason due to the chaos that is often caused by badly behaved players using TNT. Server owners may not think about optional TNT until it's too late.

paramat · 2016-06-30T19:29:46Z

IRC:
19:20 paramat nore do you mean it should be disabled by default in multiplayer?
19:20 nore paramat: yes
19:25 sfan5 paramat: the pull is fine with me if it doesnt change current behaviour
19:25 sfan5 imo TNT should be disabled by default in mp
19:30 sofar I'm fine with having it disabled by default in multiplayer as well
http://irc.minetest.ru/minetest-dev/2016-06-30#i_4641000
So +1 with t4im's code suggestion.

t4im · 2016-07-01T10:25:48Z

mods/tnt/init.lua

-	return
-end
+-- Default to enabled when in singleplayer
+local enable_tnt = minetest.setting_getbool("enable_tnt") or minetest.is_singleplayer()


You actually need to move that out into an extra if-clause checking for nil (see my other comment).
Otherwise you won't be able to disable tnt in singleplayer since it also falls back for false.

or-ing for fallback doesn't work well with boolean settings (but is great for anything else)

t4im: setting_getbool can and does return 'nil' if setting isn't seen in minetest.conf and if it does minetest.is_singleplayer() will be checked instead of using separate IF.

You missunderstood me: When you set it to false, it will still fall back to true in singleplayer. That's why you have to check it extra.

tenplus1 · 2016-07-01T13:47:18Z

@t4im better?

t4im · 2016-07-01T13:59:04Z

@tenplus1 yea this works
👍 seems good to go

paramat · 2016-07-01T23:16:48Z

mods/tnt/init.lua

-	return
+-- Default to enabled when in singleplayer
+local enable_tnt = minetest.setting_getbool("enable_tnt")
+if enable_tnt == nil and minetest.is_singleplayer() == true then


minetest.is_singleplayer() == true is redundant, just use minetest.is_singleplayer()
However if getbool returns nil in mulitiplayer then enable_tnt remains nil, t4im's suggestion works better:

local enable_tnt = minetest.setting_getbool("enable_tnt") if enable_tnt == nil then enable_tnt = minetest.is_singleplayer() end

+1 with that done.

paramat · 2016-07-01T23:18:25Z

Added line comment.

paramat · 2016-07-03T01:33:18Z

@sofar what do you think?

paramat · 2016-07-04T15:26:07Z

Looking at this i'm considering if it would be better to just disable this (as sofar suggested):

tnt.register_tnt({
    name = "tnt:tnt",
    description = "TNT",
    radius = radius,
})

Instead of disabling code inside function tnt.register_tnt(def)
The crafting of tnt:tnt will of course still need disabling to avoid an error when someone attempts to craft tnt:tnt.

So what i'm thinking is, for tidyness, have just one if enable_tnt then ... end that contains the ABM, the crafting recipe and the registration of tnt:tnt, and put this code block at the end of the file.

tenplus1 · 2016-07-05T19:52:42Z

If you disable tnt.register_tnt then the tnt:tnt_burning node will never be defined which is needed for my lucky_blocks mod; something that cannot be crafted and does not appear inside creative inventory.

paramat · 2016-07-06T07:16:21Z

Okay that's fine. Please can you group ABM and crafting recipe together within a single 'if then end' block? Then +1 and i'll merge it since it sort of has approval from sofar.

Disabling TNT shouldn't remove it altogether - Remove the TNT craft recipe, tnt:tnt node and the abm - Leave tnt:tnt_burning available for explosions in 3rd party mods - Set defaults: enabled in singleplayer, disabled in multiplayer

tenplus1 · 2016-07-06T19:48:56Z

@paramat done.

paramat · 2016-07-06T19:50:44Z

👍

paramat · 2016-07-06T19:57:07Z

And i'm assuming +1 from @sofar based on previous comments.

sofar · 2016-07-07T17:19:19Z

Yeah, I'm sure we'll run into a corner case here or there afterwards, but I'm fine with merging this as is and fixing up any remaining issues later. We're early enough in the release cycle anyway.

👍

paramat · 2016-07-07T17:20:04Z

497e6f6

paramat reviewed Jun 26, 2016
View reviewed changes

paramat reviewed Jun 27, 2016
View reviewed changes

t4im reviewed Jun 28, 2016
View reviewed changes

t4im reviewed Jul 1, 2016
View reviewed changes

paramat reviewed Jul 1, 2016
View reviewed changes

paramat added the One approval label Jul 2, 2016

paramat removed the One approval label Jul 4, 2016

TNT: Disable TNT correctly

40ffdfe

Disabling TNT shouldn't remove it altogether - Remove the TNT craft recipe, tnt:tnt node and the abm - Leave tnt:tnt_burning available for explosions in 3rd party mods - Set defaults: enabled in singleplayer, disabled in multiplayer

paramat added One approval >= Two approvals and removed One approval labels Jul 6, 2016

paramat closed this Jul 7, 2016

		@@ -4,7 +4,7 @@ local singleplayer = minetest.is_singleplayer()
		local setting = minetest.setting_getbool("enable_tnt")

Disabling TNT #1085

Disabling TNT #1085

Conversation

tenplus1 commented May 9, 2016

sofar commented May 9, 2016

C1ffisme commented May 9, 2016 • edited Loading

sofar commented May 9, 2016

C1ffisme commented May 9, 2016

sofar commented May 9, 2016

C1ffisme commented May 10, 2016

numberZero commented May 11, 2016 • edited Loading

sofar commented May 11, 2016

stujones11 commented May 11, 2016 • edited Loading

C1ffisme commented May 11, 2016

sofar commented May 11, 2016

DonBatman commented May 11, 2016

C1ffisme commented May 12, 2016

red-001 commented May 20, 2016

sofar commented May 20, 2016

paramat Jun 26, 2016

Choose a reason for hiding this comment

paramat commented Jun 26, 2016 • edited Loading

paramat commented Jun 26, 2016

paramat Jun 27, 2016

Choose a reason for hiding this comment

paramat commented Jun 27, 2016

t4im Jun 28, 2016 • edited Loading

Choose a reason for hiding this comment

tenplus1 commented Jun 29, 2016 • edited Loading

paramat commented Jun 30, 2016 • edited Loading

paramat commented Jun 30, 2016 • edited Loading

t4im Jul 1, 2016

Choose a reason for hiding this comment

tenplus1 Jul 1, 2016

Choose a reason for hiding this comment

t4im Jul 1, 2016

Choose a reason for hiding this comment

tenplus1 commented Jul 1, 2016

t4im commented Jul 1, 2016 • edited Loading

paramat Jul 1, 2016 • edited Loading

Choose a reason for hiding this comment

paramat commented Jul 1, 2016

paramat commented Jul 3, 2016

paramat commented Jul 4, 2016 • edited Loading

tenplus1 commented Jul 5, 2016

paramat commented Jul 6, 2016

tenplus1 commented Jul 6, 2016

paramat commented Jul 6, 2016

paramat commented Jul 6, 2016

sofar commented Jul 7, 2016

paramat commented Jul 7, 2016

C1ffisme commented May 9, 2016 •

edited

Loading

numberZero commented May 11, 2016 •

edited

Loading

stujones11 commented May 11, 2016 •

edited

Loading

paramat commented Jun 26, 2016 •

edited

Loading

t4im Jun 28, 2016 •

edited

Loading

tenplus1 commented Jun 29, 2016 •

edited

Loading

paramat commented Jun 30, 2016 •

edited

Loading

paramat commented Jun 30, 2016 •

edited

Loading

t4im commented Jul 1, 2016 •

edited

Loading

paramat Jul 1, 2016 •

edited

Loading

paramat commented Jul 4, 2016 •

edited

Loading