Hi, I am using resources and creating the weapons, but I soon realise that in order to implement them into my player script, I would need an if else statement that has at least 10 elif in it. I was wondering if there is any way to shorten the code or make the code cleaner.
I would like to mention that the bullets come from the player.
Below is an example is an example with only two, and to me, it’s already pretty long. I can also see it repeat itself.
# 1 second
@onready var firerate: Timer = $Firerate
var bullet_scene : PackedScene = preload("res://Scenes/bullet.tscn")
var my_gun: RangeData
if player_weapon == "Pistol":
my_gun: RangeData = load("res://pistol.tres")
elif player_weapon == "Rifle:
my_gun = load("res://rifle.tres")
if Input.is_action_pressed("shoot") and can_fire:
if player_weapon == "Pistol":
# Spawn bullet and direct it to mouse position
var new_bullet : Bullet = bullet_scene.instantiate()
var mouse_pos := (get_global_mouse_position() - global_position).angle()
# Attack stats to bullet. Pistol damage is 1
new_bullet.bullet_damage = my_gun.damage
new_bullet.global_position = global_position + Vector2(5, 0).rotated(mouse_pos)
new_bullet.global_rotation = mouse_pos
get_tree().root.add_child(new_bullet)
# Stop shooting for a second
can_fire = false
firerate.start()
elif player_weapon == "Rifle":
# Repeat
var new_bullet : Bullet = bullet_scene.instantiate()
var mouse_pos := (get_global_mouse_position() - global_position).angle()
# Attack stats to bullet. Rifle damage is 3
new_bullet.bullet_damage = my_gun.damage
new_bullet.global_position = global_position + Vector2(5, 0).rotated(mouse_pos)
new_bullet.global_rotation = mouse_pos
get_tree().root.add_child(new_bullet)
# Stop shooting for a second
can_fire = false
firerate.start()
Make a new class BaseWeapon and give it a simple base implementation of the shoot function. Something like this:
class_name BaseWeapon extends Node3D # or whatever you want to extend from
func shoot()->void:
pass
Now you can make different weapon classes that all extend from BaseWeapon. Any specifics the class needs can be declared inside this class.
Examples:
class_name Rifle extends BaseWeapon
var damage : int = 3
func shoot()->void:
print('Player fired rifle')
# your implementation here
class_name Pistol extends BaseWeapon
var damage : int = 0
func shoot()->void:
print('Player fired pistol')
In the script where you would normally do a string comparison to figure out what weapon the player is holding, you could do this:
var current_weapon : BaseWeapon = null
Now you can instantiate any weapon that derives from BaseWeapon and assign it to current_weapon. Anything that inherits from BaseWeapon is guaranteed to have a function named shoot, so that function is safe to call.
You could switch to a match statement, which is similar but reads cleaner. You could do what @TokyoFunkScene suggests, (objects) which scales better at larger sizes and keeps things more modular. There are lots of ways to solve this…
With the class scheme, I’d suggest having an “empty_hands” weapon or the like so you’re always holding something and don’t have to check the weapon for null before doing current_weapon→fire().
The class scheme also does give you some easy options for (say) handling reloading, or any other weapon-specific behaviour you might like; the match scheme can do all those things too, but it gets hairy. The class scheme will be significantly easier if you decide you want a dual_wield “weapon” that is actually two weapons; dual_wield→fire() could just call ->fire() on each of the two weapon references it has…
If you’re planning on having more than 8 weapons or complex behaviour with the weapons, I’d suggest going the class route. If not, the match scheme is more compact and fits in a single file.
Since RangeData is extending Resource you can have shoot include the player as an argument to access the scene tree
extends RangeData
func shoot(player: CharacterBody3D) -> void:
var new_bullet: Bullet = bullet_scene.instantiate()
# need scene tree for mouse position and player position
var mouse_pos := (player.get_global_mouse_position() - player.global_position).angle()
# ... and to add to the scene tree
player.get_tree().root.add_child(new_bullet)
Extending resources to only override a function feels a little strange since you will write this new script and still have to create a resource, but I am using a very similar system for weapons in my own game too.
In general try to decouple as much as possible because your player is instantiating the bullets and I think this is bad design because your player already knows too much.
Instantiation should be managed by the object itself, possibly in an agnostic way. If you need to do something after object’s ready, you could instantiate it from a global script that act as factory and implement a function inside the bullet, something like setup(various_needed_parameters_player_is_fine_too) for specific behaviour.
Then the player could have a component (a RefCounted or Resource is fine, something like WeaponComponent, BulletComponent, depending on your need) to compute specific parameters from the bullet and other things, if needed, and handlers (also RefCounted, or Node) for situation where you need to actually do something with those values.
You could use a base weapon and a dictionary with string, custom weapon resource. Store knockback, rate of fire, mag size etc in the resource, along with a packedscene for the bullet type and whatever else you want.
Then your resources contain the data, the root weapon handles shooting, recoil etc, and the bullets themselves -when they spawn- take care of dealing damage etc. No hard coding needed.
I would also use a separate dictionary of string, resource, with a custom resource for states (only bullets currently in magazine and reserve bullets). So if you change weapon when shotgun has 4 shots in it and 12 in reserve, it doesn’t come back fully loaded with full reserve when you scroll through the menus. Just populate the dictionary in ready based on what weapon resources.
Looking back at the original sample you can entirely remove the if statements, the two blocks are exactly the same code because the resources handle the damage change.
You could pack a lot of data into resource, however if you do want to keep extending weapons it would be best to leverage the built-in scripting system if for example a beam weapon is made, increasing rate of fire dynamically, higher damage with higher health etc.
But if your weapons are simple stat changes like fire rate and damage then the base resource is enough, no extensions needed.
I think you are recommending how they started this journey, previous threads @ikar had dictionaries to store weapon data, but resources will be more helpful to manage this data both for data entry and programming.
I’d recommend against trying to handle edge cases and custom functionality with more and more data, if you need custom functionality, use a new script. For example Slay the Spire uses a new script for every card effect, they efficiently leverage the scripting language where others may try to box-in effects by having a “damage” stat and “gained block” all on one resource that would spiral out of control and result in a huge if/else to process these stats where most may remain empty.
When you assign my_gun you also try to declare its type, which creates an error.
There are some things you can do to simplify your code that will make your options clearer, decision easier.
Don’t preload in variables, makes them constants.
If you’re allowing weapons to be switched, preload them.
Don’t store Resources in root. I recommend assets/resources.
When using load() or preload() don’t use paths anymore. Use UIDs. You can get the UID of any file by right-clicking on it and selecting Copy UID. That way if you move the file later, the link doesn’t get broken.
You can use an @exportDictionary to store your guns, which means you can add new ones in the editor, and the selection code becomes a single line instead of an if/elif chain or match statement.
You don’t need can_fire as a boolean variable, because you have the firerateTimer. Just check if it’s running with is_stopped().
Change the Bullet class’ bullet_damage variable to damage. It’s less redundant. You wouldn’t say “bullet bullet damage” if you were talking to me, follow the same ideas in your code.
When you create an instance of a class, you do not need to name it new_whatever. It’s just extra text.
const PISTOL = preload("uid://cotr1b0n5v4cd")
const RIFLE = preload("uid://bea8kda21i1yg")
const BULLET = preload("uid://c1lwcoiw8i7e4")
@export var guns: Dictionary[String, RangeData] = {
"Pistol": PISTOL,
"Rifle": RIFLE,
}
var my_gun: RangeData
@onready var firerate: Timer = $Firerate # 1 second
###Assuming this is inside a function.
my_gun = guns.player_weapon
if Input.is_action_pressed("shoot") and firerate.is_stopped():
# Spawn bullet and direct it to mouse position
var bullet: Bullet = BULLET.instantiate()
var mouse_pos := (get_global_mouse_position() - global_position).angle()
# Attack stats to bullet.
bullet.bullet_damage = my_gun.damage
bullet.global_position = global_position + Vector2(5, 0).rotated(mouse_pos)
bullet.global_rotation = mouse_pos
get_tree().root.add_child(bullet)
# Stop shooting for a second
firerate.start()
Just refactoring your code makes your problem go away, and your question of how to deal with it become immaterial.
EDIT: I’d also recommend moving the fire code to the bullet. Then you could do this:
if Input.is_action_pressed("shoot") and firerate.is_stopped():
# Spawn bullet and direct it to mouse position
var bullet: Bullet = BULLET.instantiate()
var mouse_pos := (get_global_mouse_position() - global_position).angle()
bullet.fire(my_gun.damage, global_position, mouse_pos)
# Stop shooting for a second
firerate.start()
And do the rest of the calculations in the bullet and add it to the scene tree there.
I meant a dictionary with string key and resource value, not that each weapon itself should be a dictionary. Then the weapon could do whatever it normally does but with the speed or mag size, whatever the resource says, without any elses or ifs. Using a dictionary is mostly for ease of use and personal preference though i guess.
How do you mean each effect should have its own script? For example for a gun you would combine multiple scripts in various combinations?
Would it be better to have a script for each gun, but only one bullet script. Or, could I have several bullets, but they have different names. (Example: pistol_bullet, rifle_bullet, etc.)
How would I get the bullet to move if it’s extending Node and not Area2D?
How do I select the class?
Below is the code. Control + f “##” to see the issues
It’s somewhat an example.
class_name BaseWeapon extends Node
func shoot(player) -> void:
pass
# In a different script
## Techincally, this script is in bullet. I was wondering if that's okay or should I separate the guns and keep the bullet in it's own script.
class_name Pistol extends BaseWeapon
var bullet_scene : PackedScene = preload("res://Scenes/bullet.tscn")
var damage : int = 1
var bullet_speed : int = 500
var travel_distance : float = 0.0
var max_range : int = 1500
func _physics_process(delta: float) -> void:
# Increases the further it goes until it hits max range. I don't want 1000s of bullets
travel_distance += bullet_speed * delta
if travel_distance > max_range:
queue_free()
func shoot(player) -> void:
var bullet : BaseWeapon = bullet_scene.instantiate()
var mouse_pos = (player.get_global_mouse_position() - player.global_position).angle()
## This causes an error since it's not extending Area2D. The script is on Node. I want the bullet to keep moving
var direction = mouse_pos
position += direction * bullet_speed * delta
player.get_tree().root.add_child(bullet)
func _on_body_entered(body: Node2D) -> void:
# This is for the enemy
if body.has_method("take_damage"):
body.take_damage(bullet_damage)
queue_free()
In player script
@onready var player: CharacterBody2D = $"."
var current_weapon : BaseWeapon = null
var can_fire : bool = true
# Just ignore the can_fire
func _ready() -> void:
## I want it to equal class, but what if I have different classes? Is there any way to search for that? Store classes?
current_weapon = ...
func _physics_process(delta: float) -> void:
if Input.is_action_pressed("shoot") and can_fire:
current_weapon.shoot(player)
I want to try out classes first, but if it gets too complicated, I might revert back to resources.
You could have many different bullet scripts if need be, extending scripts and scenes is built to be easy in Godot.
You really really should extend Node2D, there is very little reason to extend Node on it’s own.
Not sure i understand the question but there are certainly some things wrong with this implementation that may be relevant.
Currently you player script has a var current_weapon: BaseWeapon, but this base weapon script appears to hold one bullet, not weapon information. I say this mostly because of it’s physics process which will automatically free it after some distance, once that time is up you will be left with an invalid current_weapon
The BaseWeapon @TokyoFunkScene recommends does not delete itself after a set time, it is in charge of spawning the bullets and assigning their damage similar to your first sample, or the sample I posted for a RangeData resource.
When you have a weapon type you want to use it can be added as a child to the player, and you must either assign current_weapon to it, for example if the weapon is a pickup, and/or @export the var current_weapon if it’s a starting weapon.
With nodes you would have to point to the new base weapon i.e. current_weapon = new_weapon, there are a few ways to do this, if you add all the weapon types as children of the player you can swap between by node path current_weapon = $Weapons/Rifle. If the weapons are picked up and/or destroyed somehow you will have to instantiate the weapon, add it to a child of something, and have the player point to it.
With resources you will load the resource onto the current weapon, very similarly current_weapon = load(new_weapon_path).
Sorry, I’m new to Godot, so I don’t get what you’re saying. For example, what is current_weapon? what is new_weapon? Swap between node path? How would that be done without a bunch of if else?
These are the same statement so I hope they give the same error.
Your ‘Nil’ error is the fact that current_weapon doesn’t equal anything on the player, if you leave out the = sign it will be null by default which triggers this error.
Though you likely have larger design problems. Your RangeClass should not represent one single bullet since those will likely be deleted and thus current_weapon invalidated quite often. The function shoot instantiating itself is strange and won’t work since @onready var bullet: RangeClass isn’t a scene resource.
I’d still recommend Resources, I don’t think you’ll get the most out of Nodes for this and it will complicate things further.
I’ve given up on classes. I will go back to resources.
class_name RangeResouce
extends Resource
@export var range_name : String = “Pistol”
@export var damage : int = 1
@export var bullet_speed : int = 500
@export var travel_distance : float = 0.0
@export var max_range : int = 1500
Question, do I apply the RangeResource to player? I just want to make sure I’m doing it correctly
Also, what if I have different looking bullets? How would I change between them? I’ve provided an example
# Both have _physics_process() and _on_body_entered()
#Main Bullet
class_name bullet
extends Area2D
#Large bullet
class_name LargeBullet
extends Area2D
I know you said this, but what is new_weapon_path?
Edit:
I’m guessing var new_weapon_path : PackedScene = preload(“res://Scenes/bullet.tscn”)
But again, I have res://Scenes/largebullet.tscn
Yes, I’d follow your previous examples with var current_weapon: RangeResource
You can add this to your range resource
# in range_resource.gd
class_name RangeResouce
extends Resource
@export var range_name : String = “Pistol”
@export var damage : int = 1
@export var bullet_speed : int = 500
@export var travel_distance : float = 0.0
@export var max_range : int = 1500
@export_file("*.tscn") var bullet_scene_path: String = "res://my_normal_bullet.tscn"
Then load the bullet from the path provided by the resource
# in player.gd
@export var current_weapon: RangeResource
func shoot() -> void:
var new_bullet := load(current_weapon.bullet_scene_path).instantiate()
new_bullet.damage = current_weapon.damage
# etc ...
Your current_weapon must take a RangeResource in, not a bullet scene. You would load the created resource.
var new_weapon_path: String = "res://my_weapon_pistol.tres"
current_weapon = load(new_weapon_path)
Maybe this got lost in the weeds too, but after you make the resource script you will create resources of that type by right click the filesystem panel and selecting “Create New → Resource” then selecting your class name RangeResource. The generated .tres file will have all of the @export variables as properties.
Invalid access to property or key ‘range_scene_path’ on a base object of type ‘Nil’.
class_name RangeResouce
extends Resource
@export var range_name : String = "Pistol"
@export var damage : int = 1
@export var firerate : float = 1.0
@export var fire_speed : int = 500
@export var travel_distance : float = 0.0
@export var max_range : int = 1500
@export_file("*.tscn") var range_scene_path: String = "res://Scenes/Player_Projectile/bullet.tscn"
In player script
@export var stat : RangeResource
var new_weapon : string = "res://GlobalAndResource/Pistol.tres"
var current_weapon = load(new_weapon)
var can_fire : bool = true
func shoot() -> void:
if Input.is_action_pressed("shoot") and can_fire:
# Just noticed this. Removing current_weapon still brings up a similar error
# Edit: Actually, this should be current_weapon? new_weapon?
var new_bullet = load(stat.range_scene_path).instantiate()
var mouse_pos := (get_global_mouse_position() - global_position).angle()
new_bullet.damage = stat.damage
new_bullet.global_position = global_position + Vector2(5, 0).rotated(mouse_pos)
new_bullet.global_rotation = mouse_pos
get_tree().root.add_child(new_bullet)
This script seems much better, if stat is nil you must assign it, as an @export you can do this from the editor. You can also assign it as part of any other weapon switching system you create
