minetest-mods / ccompass

https://forum.minetest.net/viewtopic.php?f=9&t=17881
Other
5 stars 9 forks source link

Propose multi air #17

Closed SwissalpS closed 3 years ago

SwissalpS commented 4 years ago

If target position is in vacuum or is no longer in air, compass would place player at 160 nodes above target no matter what is there, including lava. This fixes that.

Also proposal for multiple allowed and configurable nodes that must be above target node.

tag:not-tested

SwissalpS commented 4 years ago

I intentionally did not check if the player can stand at target to allow 'air-drops'. Maybe there should be a check for dangerous nodes though :/

SwissalpS commented 4 years ago

found the time for a quick test. Loads and destination in water worked. Player was tp-ed to the actual location opposed to on top of water or in middle of rock as it did before the change.

bell07 commented 4 years ago

Great Idea. But I like more generic ways. I think the better way is to check the "drawtype" from node definition:

drawtype = "airlike", "liquid", "flowingliquid", "plantlike", "plantlike_rooted" should match all needs.

But the lava is "liquid" too, so may an second check for "damage_per_second" is useful.

SwissalpS commented 4 years ago

yes, the exceptions made me go the less generic route. Also this allows to adjust better to the game type a server is aiming for.

bell07 commented 4 years ago

Maybe both, the generic way by default + the exceptions table to override the generic result? ccompass.air_nodes["bad_node"] = false is allowed too.

I like if the mod does work out-of the-box in any game without adjustments.

Tried the next check with qa_block mod

for name, def in pairs(minetest.registered_nodes) do
if (not def.damage_per_second or def.damage_per_second == 0) and ( def.drawtype == "airlike" or def.drawtype == "liquid" or def.drawtype == "flowingliquid" or def.drawtype == "plantlike" or def.drawtype == "plantlike_rooted" ) then
        print(name)
    end
end

found the next nodes in whynot game:

farming:carrot_5    
homedecor:table_legs_wrought_iron   
canned_food:rhubarb_jam 
wielded_light:1 
canned_food:blueberry_jam   
farming:raspberry_1 
default:grass_2 
farming:cotton_2    
default:dry_grass_4 
farming:tomato_3    
flowers:tulip_black 
farming:cotton_5    
farming:cucumber_2  
farming:pineapple_6 
default:sand_with_kelp  
canned_food:canned_pumpkin  
default:coral_cyan  
canned_food:canned_cucumber 
farming:barley_4    
wielded_light:2 
farming:pepper_5    
default:apple_mark  
farming:pineapple_1 
farming:beetroot_2  
farming:carrot_8    
farming:melon_7 
farming:chili_2 
farming:cucumber_1  
farming:cocoa_3 
farming:barley_6    
butterflies:hidden_butterfly_violet 
farming:wheat_4 
ethereal:orange 
farming:hemp_4  
air 
farming:pineapple_4 
farming:oat_1   
farming:cotton_8    
default:dry_shrub   
farming:chili_7 
maple:maple_sapling 
farming:potato_3    
farming:tomato_1    
subspacewalker:subspace 
homedecor:cobweb_plantlike  
farming:mint_1  
farming:oat_2   
farming:carrot_6    
default:water_source    
farming:blueberry_2 
farming:hemp_1  
default:grass_5 
farming:grapes_7    
farming:tomato_4    
default:fern_3  
fireflies:firefly_bottle    
farming:cotton_6    
flowers:tulip   
farming:corn_5  
farming:bottle_ethanol  
vessels:glass_bottle    
wielded_light:13    
farming:pineapple   
farming:wheat_6 
farming:melon_2 
mesecons_blinkyplant:blinky_plant_on    
farming:coffee_1    
farming:barley_5    
wielded_light:8 
farming:cocoa_2 
farming:wheat_2 
default:pine_bush_stem  
canned_food:canned_onion    
farming:pineapple_2 
wielded_light:12    
farming:chili_1 
farming:potato_1    
farming:tomato_7    
farming:wheat_3 
farming:oat_5   
default:junglegrass 
homedecor:utility_table_legs    
farming:melon_3 
farming:pepper_ground   
default:grass_3 
farming:pea_2   
default:river_water_source  
farming:pepper_4    
farming:rice_2  
mesecons_powerplant:power_plant 
farming:potato_4    
default:pine_bush_sapling   
farming:tomato_2    
default:water_flowing   
mesecons_random:ghoststone_active   
butterflies:hidden_butterfly_red    
farming:blueberry_3 
default:grass_4 
flowers:rose    
canned_food:canned_garlic_cloves    
farming:onion_4 
farming:mint_4  
farming:corn_4  
farming:tomato_5    
farming:beetroot_1  
farming:rice_3  
default:apple   
default:bush_sapling    
farming:hemp_rope   
farming:oat_7   
farming:cocoa_1 
vessels:drinking_glass  
farming:pea_4   
farming:oat_6   
farming:coffee_4    
farming:melon_5 
farming:pepper_2    
farming:pumpkin_4   
farming:potato_2    
farming:tomato_8    
wielded_light:4 
flowers:mushroom_brown  
default:aspen_sapling   
farming:beanpole    
canned_food:canned_corn 
flowers:dandelion_white 
default:acacia_sapling  
default:blueberry_bush_sapling  
farming:pea_5   
farming:grapes_6    
farming:coffee_3    
princess:gold_chandelier    
homedecor:table_legs_brass  
vessels:steel_bottle    
wielded_light:9 
farming:beanpole_1  
flowers:mushroom_red    
default:sapling 
farming:wheat_7 
farming:corn_3  
canned_food:canned_tomato_plus  
tnt:boom    
farming:tomato_6    
farming:rhubarb_3   
cloudlands:cobweb   
wielded_light:14    
farming:grapebush   
farming:pumpkin_3   
default:marram_grass_2  
farming:pineapple_5 
farming:beetroot_3  
farming:corn_8  
canned_food:canned_mushrooms_plus   
farming:rice_7  
farming:pea_3   
canned_food:raspberry_jam   
canned_food:canned_garlic_cloves_plus   
canned_food:canned_chili_pepper_plus    
canned_food:canned_chili_pepper 
canned_food:blackberry_jam  
canned_food:grape_jam   
canned_food:canned_potato_plus  
canned_food:canned_potato   
canned_food:apple_jam   
canned_food:rose_jam    
canned_food:canned_peas 
canned_food:canned_pineapple    
canned_food:melon_jam   
canned_food:canned_onion_plus   
canned_food:wild_blueberry_jam  
canned_food:canned_beans    
canned_food:canned_carrot_plus  
canned_food:canned_carrot   
canned_food:canned_beetroot_plus    
canned_food:canned_beetroot 
canned_food:canned_cucumber_plus    
canned_food:canned_tomato   
canned_food:dandelion_jam   
farming:wheat_8 
wielded_light:7 
farming:raspberry_2 
farming:carrot_3    
farming:cabbage_3   
mtfoods:apple_cider 
mobs:cobweb 
default:grass_1 
farming:onion_2 
default:marram_grass_3  
farming:beanpole_5  
butterflies:hidden_butterfly_white  
3d_armor_stand:top  
farming:hemp_oil    
farming:cabbage_4   
fake_fire:ice_fire  
farming:chili_4 
butterflies:butterfly_red   
default:river_water_flowing 
farming:rye_1   
farming:rice_1  
farming:rose_water  
farming:salt    
farming:cabbage_6   
farming:mint_3  
farming:mint_2  
farming:rice_8  
farming:rice_6  
farming:rice_5  
farming:oat_8   
farming:oat_4   
farming:oat_3   
farming:rye_8   
farming:rye_7   
farming:rye_6   
farming:rye_4   
farming:rye_2   
farming:chili_6 
farming:chili_5 
farming:chili_3 
farming:pumpkin_7   
farming:beetroot_4  
handholds:climbable_air 
canned_food:canned_mushrooms    
farming:pea_1   
farming:pineapple_8 
farming:pineapple_3 
farming:pepper_3    
farming:onion_5 
farming:onion_3 
farming:garlic_5    
farming:garlic_4    
farming:garlic_3    
farming:garlic_1    
farming:hemp_7  
farming:hemp_6  
farming:hemp_3  
default:dry_grass_5 
farming:barley_7    
farming:barley_3    
farming:barley_2    
farming:barley_1    
farming:grapes_8    
farming:grapes_5    
farming:grapes_4    
farming:grapes_3    
farming:grapes_2    
farming:grapes_1    
farming:beanpole_4  
farming:beanpole_2  
farming:rhubarb_2   
farming:blueberry_1 
farming:raspberry_4 
farming:cocoa_4 
farming:beetroot_5  
farming:pumpkin_6   
farming:pumpkin_5   
farming:pumpkin_2   
farming:pumpkin_1   
farming:melon_6 
farming:melon_1 
farming:coffee_5    
farming:coffee_2    
farming:garlic_2    
farming:corn_7  
farming:corn_6  
farming:corn_2  
farming:cucumber_4  
farming:cucumber_3  
farming:pepper_1    
farming:blueberry_4 
wielded_light:5 
farming:trellis 
farming:hemp_8  
farming:carrot_1    
default:marram_grass_1  
default:junglesapling   
farming:cotton_1    
farming:cabbage_1   
farming:cabbage_2   
default:pine_sapling    
farming:carrot_4    
default:dry_grass_2 
fireflies:hidden_firefly    
cloudlands:air  
flowers:chrysanthemum_green 
farming:cotton_4    
farming:rye_3   
farming:raspberry_3 
wielded_light:6 
default:coral_pink  
default:emergent_jungle_sapling 
ignore  
doors:hidden    
default:fern_2  
flowers:viola   
farming:cabbage_5   
default:dry_grass_3 
farming:carrot_7    
default:acacia_bush_sapling 
fireflies:firefly   
wielded_light:10    
wielded_light:3 
wielded_light:11    
farming:cotton_7    
default:dry_grass_1 
farming:beanbush    
butterflies:butterfly_white 
farming:beanpole_3  
farming:hemp_5  
princess:gold_chandelier_unlit  
mesecons_blinkyplant:blinky_plant_off   
farming:wheat_5 
default:fern_1  
farming:melon_4 
farming:corn_1  
flowers:dandelion_yellow    
default:coral_green 
butterflies:butterfly_violet    
farming:wheat_1 
farming:onion_1 
farming:rice_4  
flowers:geranium    
default:large_cactus_seedling   
farming:chili_8 
farming:cotton_wild 
default:acacia_bush_stem    
farming:cotton_3    
default:papyrus 
farming:rye_5   
farming:carrot_2    
farming:rhubarb_1   
farming:pineapple_7 
farming:hemp_2  
default:bush_stem   

From this list the "ignore" should be excluded hard-coded. All other from the list are fine to be teleported in.

Please check which nodes are found in your game. Maybe additional parameter with defaults above?

ccompass_air_drawtypes = "airlike", "liquid", "flowingliquid", "plantlike", "plantlike_rooted" 
ccompass_air_allow_damage = 0
ccompass_air_nodes = 
ccompass_air_nodes_exclude = 
SwissalpS commented 4 years ago

I ran that snippet on this mod collection and here is the complete list.

There are some questionable ones like: tnt:boom, scifi_nodes:tower, homedecor:table_legs_wrought_iron, default:sand_with_kelp

Then there are some missing like bridger:scaffolding and vacuum:vacuum.

I'm thinking instead of adding black and white lists, provide a public function that per server mods can override? function ccompass.is_safe_target(pos) .... end

Edit: that would also allow for per server decision on whether to check combinations of head, foot and under nodes. Depends on what your vision of this mod is :)

SwissalpS commented 4 years ago

I've added more generic solution. It still does not entirely satisfy me as now, if there is lava at destination, player is placed above it instead of in it, which mostly is not safe :/

Edit: then again, is it the scope of this mod to check against changes and grief attempts?

SwissalpS commented 3 years ago

now head, foot and under nodes are checked. Also added more configurable options.

bell07 commented 3 years ago

Did a code review today. See next issues in implementation:

  1. The restricted nodes with ccompass.restrict_target is for calibration only. Teleport should be still allowed. Please remove the check from is_safe_target_under()

  2. The loop contains 3x get_node(). 2 of them are redundant from second loop-step. You can cache them in internal variables.

  3. Teleporting above lava still can happens, if you have air, air, air, lava. I think the "under" should be checked for "solid / not airlike" and "not damaging".

SwissalpS commented 3 years ago

Thanks for the review. 1) ok, done (not yet pushed to github) 2) are you sure, the only place I see three occurences of get_node() in a loop, is in teleport_above() and each is checking a different node. 3) left that open for air-drops. Will change it as that isn't as important.

SwissalpS commented 3 years ago

Done aka ready for next review. It is still possible to land on radioactive nodes, which I think is fine as player can be wearing hazmat suit.

bell07 commented 3 years ago

Found issue with if the node bellow the target is removed the teleportation does not work anymore. Previously the player was teleported into air and is fallen down to the ground. Now we need to search for the ground bellow...

bell07 commented 3 years ago

Did 2 additional changes, to allow to teleport bellow the target and to cache the get_node().name values.

Still unhappy with the case the teleportation into house is not possible anymore, the player is now teleported on the roof.

But no time to solve it ...

I am unsure if the whole changes are the right way. I think the compass should always teleport to the target without any checks. The logic that stores the target into the compass should check for valid target.

The previous check was for the "Schnitzeljagd" mod to teleport "on the ground" without merging the whole map to find right positions ...

SwissalpS commented 3 years ago

The logic that stores the target into the compass should check for valid target.

that is insufficient as the target node can have changed in the meantime.

Thanks.