Restore technic.charge_tools for compatibility

[S-S-X]

While looking for mods that might require updates for new power tools API I've noticed https://content.minetest.net/packages/gpcf/technictrain/ which required removed public function technic.charge_tools: https://git.bananach.space/technictrain.git/tree/init.lua#n44

I've removed (made local) said function myself here: 968c64b

Should this be restored?
Or ask external machines to utilize new technic.set_RE_charge(stack, charge) / technic.get_RE_charge(stack) functions?

First option is to provide compatibility (and increase API footprint which is bad for documentation and maintenance).
Second option requires either another compatibility override (which might break things if original mod is updated) or updating mod code.

[S-S-X]

Personal opinion: function should not be restored because it is not really necessary for functionality and API filled with unnecessary methods is bad API.

[OgelGames]

Yeah, I think that would fall into the "never intended to be API" category, so I don't think it should be restored.

It definitely should be mentioned somewhere though, probably in the release changelog. (and a technic plus migration guide maybe?)

EDIT: Added to the release changelog here: https://github.com/mt-mods/technic/releases/tag/1.1.0

[S-S-X]

It definitely should be mentioned somewhere though, probably in the release changelog. (and a technic plus migration guide maybe?)

That function actually was not documented and looked pretty much like thing for internal use only, that technic train was only thing I could find when I started looking around for it. I don't think there's a lot of migration happening with that one but it is available on original technic mod.
Could be mentioned somewhere but more important is getting actual current documentation updated and while doing that also look for redundant functions for cleanup (documenting as do not use this without any good explanation what it does).

[OgelGames]

Compared the technic namespace between minetest-mods/master and mt-mods/master, only other removed function is technic.smelt_item, which was not used at all, and was only removed recently in 03738e3 (not in any releases yet).

[OgelGames]

also look for redundant functions for cleanup

This is probably useful then:

All the functions in technic namespace as of commit 713409b

activate_network
add_network_branch
add_network_node
build_network
can_insert_unique_stack
chests.change_allowed
chests.digiline_effector
chests.get_formspec
chests.get_inv_items
chests.get_receive_fields
chests.log_inv_change
chests.move_inv
chests.register_chest
chests.send_digiline_message
chests.sort_inv
chests.update_formspec
config.get
config.get_bool
config.get_flags
config.get_int
config.get_names
config.get_np_group
config.remove
config.set
config.set_bool
config.set_np_group
config.to_table
config.write
create_network
default_can_insert
disable_machine
EU_string
getter
get_cable_tier
get_max_lag
get_or_load_node
get_recipe
get_RE_item_load
get_timeout
handle_machine_pipeworks
handle_machine_upgrades
insert_object_unique_stack
is_overloaded
is_tier_cable
machine_after_dig_node
machine_can_dig
machine_inventory_move
machine_inventory_put
machine_inventory_take
merge_networks
network2pos
network2sw_pos
network_infotext
network_node_on_dignode
network_node_on_placenode
network_run
new_default_tube
overload_network
pos2network
pretty_num
radiation_callback
refill_RE_charge
register_alloy_furnace
register_alloy_recipe
register_base_machine
register_battery_box
register_cable
register_can
register_centrifuge
register_compressor
register_compressor_recipe
register_electric_furnace
register_extractor
register_extractor_recipe
register_freezer
register_freezer_recipe
register_generator
register_grinder
register_grinder_recipe
register_machine
register_power_tool
register_recipe
register_recipe_type
register_separating_recipe
register_solar_array
register_tier
remove_network
remove_network_node
reset_overloaded
send_items
set_default_timeout
set_RE_item_load
set_RE_wear
swap_node
sw_pos2network
sw_pos2tier
touch_node
trace_node_ray
trace_node_ray_fat
tube_inject_item
worldgen.gettext

And the WE command to get that:

//lua for a,b in pairs(technic)do if type(b)=="function"then print(a)elseif type(b)=="table"then for c,d in pairs(b)do if type(d)=="function"then print(a.."."..c)end end end end

[S-S-X]

I do like to use cli tools bit more as for example simple grep -nr 'function.*technic.*\|technic.*=.*function' gives pretty good results with file name, line number and code snippet. And add -A, -B or -C for context when needed.

[BuckarooBanzay]

My opinion: don't restore it

Worst case something like that could end up in some kind of "technic-shim" or "technic-compat" mod to make it backwards compatible (maintaining that would be a nightmare though)

[S-S-X]

Compatibility wrapper could be added for said train mod support but better of course would be to update mod itself, something like this with yelling about deprecated calls would probably work just fine:

function technic.charge_tools(meta, batt_charge, charge_step)
	local src_stack = meta:get_inventory():get_stack("src", 1)
	local def = src_stack:get_definition()
	if not def or not def.max_charge or src_stack:is_empty() then
		return batt_charge, false
	end
	local charge = technic.get_RE_charge(src_stack)
	charge_step = math.min(def.max_charge - charge, charge_step)
	technic.set_RE_charge(src_stack, charge + charge_step)
	meta:get_inventory():set_stack("src", 1, src_stack)
	return batt_charge, (def.max_charge == charge + charge_step)
end

Maybe I'll try to see if there's some simple way to make that happen in mod...

[OgelGames]

That's probably a good idea actually, because without that function that mod would crash, which is something we should be avoiding whenever possible. 👍

[S-S-X]

Now after #275 this can be done better with technic_get_charge/technic_set_charge:

function technic.charge_tools(meta, batt_charge, charge_step)
	local src_stack = meta:get_inventory():get_stack("src", 1)
	local def = src_stack:get_definition()
	if not def or not def.technic_max_charge or src_stack:is_empty() then
		return batt_charge, false
	end
	local new_charge = math.min(def.technic_max_charge, def.technic_get_charge(src_stack) + charge_step)
	def.technic_set_charge(src_stack, new_charge)
	meta:get_inventory():set_stack("src", 1, src_stack)
	return batt_charge, (def.technic_max_charge == new_charge)
end

Add to compatibility shims and include log warning about deprecated API function.

Warning because function is bad for API as it enforces inventory list name and inventory slots.
Therefore it should not be included in official API but is good addition for compatibility until mods start using new API which is now also available and should be used with minetest-mods technic.