I would recommend against freeing and reinstancing smaller objects that are not proving to be a performance issue in memory, and go with hiding and showing them, and skip their processing where it still might be active.
The reason is, that while instancing can be fast enough that it might not interfere with your frame's budget, adding nodes to the tree sometimes isn't. It can sometimes eat up a lot of frame time for just a couple of nodes, causing a game to stutter a few frames.
The problem in this snippet is that the variables used to hold the weapons are local. There is no memory of the previous references. So it looks like its creating ~60 instances of each weapon a second.
These are orphaned instances, and when you run long enough the game should get overloaded and come to a halt. They're not in the tree, so you won't see them.
Check your orphan count. In the montiors or via code:
The weapons being seen are the ones being added to the SceneTree, which happen only when a button is just pressed. Of the two instances, during input one is added to the tree, and the other is freed. These are references to the previous weapons.
What you're likely seeing are previously leaked instance, not the failure of
queue_free() to free nodes.
If you go with this instancing/freeing approach, managing some variables at a higher scope would solve the problem.
# v3.2.4 (Psuedo-code)
const UNARMED = 0
var current_slot : int = UNARMED
var current_weapon : Node
if(Input.is_action_just_pressed("slot 1") and current_slot != 1):
if(Input.is_action_just_pressed("slot 2") and current_slot != 2):
func swap_weapon(p_slot : int, p_weapon : PackedScene) -> void:
current_slot = p_slot
current_weapon = p_weapon.instance()