Input called multiple times.

Maybe use a bool is_reloading and set it to true the first time when trying to fire with empty magazine and then check if firing and magazine is empty if it is already reloading.

if Input.is_action_pressed("Fire"):
    weapon_manager.fire_weapon()

This is going to run really quickly unless there’s some constraint (e.g. like a cool down timer) in fire_weapon

I guess I misunderstood your question.
So you want to have the player stop pressing fire and then pressing it again to trigger a reload when magazine is empty?
But when he still holds fire when the magazine becomes empty no reload should be triggered?

In that case I would create something like that:

if Input.is_action_pressed("Fire"):
		if weapon is !singleshot_weapon and magazine ammo > 0:
                    fire

if Input.is_action_just_pressed("Fire"):
                if magazine ammo 0:
                       reload
                elif weapon is singleshot_weapon:
                       fire

Something like that.

verypogu The method fire_weapon() shouldn't be calling reload() because firing is not reloading. Instead, the function that handles the input should call either one or the other depending on the ammo state. This seems like a nitpick but, as you can see, it confused you enough to lose control over what is happening. Let your functions do only what their name suggests. It'll make your code more readable and its logic easier to follow.

Also get rid of the abstract weapon_manager because it doesn't represent anything in the real world. Soldiers do not carry any "weapon managers" into fight. They only have weapons that can fire and reload. So your class structure should represent that in the simplest possible way, without funky abstract go-betweens that produce bugs

verypogu The WeaponManager is abstract and thus doesn't really represent anything that exists in your simulated world. It just complicates your modeling by introducing more objects than necessary and with that - more potential bugs. All in the name of looking like you're doing some fancy oo stuff.

Keep it simple: a weapon should fire and count ammo and the player should equip weapons, reload them and press the trigger. Just like in real world. "Managers" in oo programming are like middle managers in corporations - useless, annoying busybodies that do more harm than good.

verypogu
Here's how I'd model a weapon that doesn't need a manager, doctor or lawyer. It can work as a single shot or auto weapon with adjustable fire rate and reloading time and also reloads itself when out of ammo. All that player object now needs to do is call trigger_pressed() or trigger_released() when the fire button is pressed/released:

class_name Weapon extends Node

# stats
var fire_delay: float = .2
var reload_delay: float = 1.2
var ammo_max: int = 10

# current state
var firing: bool = false
var reloading: bool = false
var delay: float # firing or reloading timer
var ammo: int = ammo_max

func _process(dt):
	delay = delay-dt if delay >= 0.0 else delay
	if delay < 0.0:
		if reloading:
			ammo = ammo_max
			print("RELOAD DONE")
			reloading = false
			delay = 0.0
			
		elif firing:
			ammo -= 1
			print("FIRE (ammo left: " , ammo, ")")
			if ammo:
				delay = fire_delay
			else:
				reload()

func trigger_pressed():
	if ammo:
		delay = 0.0
		firing = true
	else:
		reload()
	
func trigger_released():
	firing = false
	
func reload():
	if not reloading:
		reloading = true
		delay = reload_delay
		print("RELOADING...")

verypogu Do you think any of these would be a good example for when to use a Manager?

Nope. All of them sound completely redundant. Looks like something from "everything is an object" school of thinking which I don't subscribe to. Classes are meant to represent instantiable types. If you don't need an instantiable type - you don't need a class.

The code can always be organized so that's not encumbered with this "manager" way of thinking. If Experience and Upgrades (from your examples) are properties of a player then the Player class should be responsible for managing them. If Enemies are properties of a level then the Level class should manage their spawning etc. No need to invent some "manager" intermediaries and overcomplicate the system with their existence.

I mean just look at your problem in this thread. Has using a weapon manager made things simpler to handle and debug or did it actually contribute to the tangle you ended up in?

In problem solving, it's always advisable to follow the principle of Occam's razor. In the context of class system design that'd mean introducing only the classes that are absolutely necessary.

verypogu Having reusable components is always good, but here too you have to take care not to overdo and overdesign it. Just because something can be made a component, it doesn't mean it should. Exercise economy here as well and be sure that introduction of a component actually simplifies things. Similarly to having too many classes, too many components can create chaos.

Unlike some other engines, Godot doesn't have an explicit ECS system. Its equivalent of a component is a scene. So everything that should function as a reusable, self contained component - should be a scene.

Your weapon example sure sounds like an overkill. You insist on turning every tiny bit of functionality into a component, just because you can. This is not a very wise design choice as you'd end up with too many of too small components. This is cumbersome to manage mentally, and it also makes statements in your code long and repetitive.

Godot's architecture is based mainly on node class inheritance. In many cases it may be better to pack the shared functionality into a base class instead of scattering it among the components. But decision on how to approach it really depends on the system you're trying to model and your stance on the eternal oo conundrum: Is-a vs Has-a

To go back to your weapon example, that's definitely too many tiny components. This would be better handled via a base class that holds all of the ranged weapon common functionality. Also, no need to use the word component in component names. It just contributes to annoying longwindedness. Now if you throw in several nested levels of such components and a "manager" or two on top of it, then you'll likely end up having to write a lot of extremely long and similar looking statements just to address some basic properties. For example:

player.weapon_manager.equipped_weapon.ammunition_component.current_ammo -= 1
enemy.health_manager.current_hp -= player.weapon_manager.equipped_weapon.ranged_weapon_stats_component.damage

Doing this a lot will make your code excruciatingly hard to read and understand. Especially when it starts to grow larger. Instead, you should design your class/components structure as well as your naming conventions so it results in simple, easy to read and understand statements like this:

player.weapon.ammo -= 1
enemy.hp -= player.weapon.damage