Looking for Advice/Improvements

Development Programming & Scripting C++
1

I have made a weapon switching system its not 100% done like the models are not in place but the basic switching is there. I am going to put my code of how i made it to switch and anything that you notice that is inefficient and can cause a problem could you please tell me and also include a way of how i can make it better or just pseudo code it as i dont want to be spoon feed so i can learn something from it. Also could you add a reason why i would need to change it like what problems could it cause etc.

first i spawn my primary and secondary guns at BeginPlay() i set the secondary gun to hidden and primary gun to visible and i also set my current gun to the primary weapon as thats the starting gun i want.



	PrimaryWeapon = GetWorld()->SpawnActor<AWeaponBase>(PrimaryWeaponBlueprint);
	PrimaryWeapon->AttachToComponent(Mesh1P, FAttachmentTransformRules(EAttachmentRule::SnapToTarget, true), TEXT("WeaponSocket"));
	PrimaryWeapon->SetActorHiddenInGame(false);
	PrimaryWeapon->Character = this;

	SecondaryWeapon = GetWorld()->SpawnActor<AWeaponBase>(SecondaryWeaponBlueprint);
	SecondaryWeapon->AttachToComponent(Mesh1P, FAttachmentTransformRules(EAttachmentRule::SnapToTarget, true), TEXT("WeaponSocket"));
	SecondaryWeapon->SetActorHiddenInGame(true);
	SecondaryWeapon->Character = this;

	CurrentWeapon = PrimaryWeapon;

In my character class i make a ChangeWeapon() Method and add the code to allow the primary and secondary guns to change.



void ALearnProjectCharacter::ChangeWeapon() {
	if (CurrentWeapon == PrimaryWeapon || CurrentWeapon == nullptr) {
		CurrentWeapon = SecondaryWeapon;
		PrimaryWeapon->SetActorHiddenInGame(true);
		SecondaryWeapon->SetActorHiddenInGame(false);
		UE_LOG(LogTemp, Warning, TEXT("SecondaryEquipped"))
	}
	else if(CurrentWeapon == SecondaryWeapon) {
		CurrentWeapon = PrimaryWeapon;
		PrimaryWeapon->SetActorHiddenInGame(false);
		SecondaryWeapon->SetActorHiddenInGame(true);
		UE_LOG(LogTemp, Warning, TEXT("PrimaryEquipped"))
	}
}

Then i have an OnFire() Method and it looks for the CurrentGun when switching weapons the current gun will change as you can see from the code and it will fire the correct fire method for the gun



void ALearnProjectCharacter::OnFire() {
	CurrentWeapon->WeaponFire();
}

That’s basically the code apart from the variables which there is no need to add this all does work but im just looking for alternate ways to make my code more efficient and remove unnecessary code.
Thanks for all reply’s in advance.

2

In terms of “inefficient”, this is code that will run probably once every few hundred or thousand frames at the most. You still need to be careful about submitting too much work to the renderer, though, but all you’re really doing is setting one mesh hidden, and the other mesh not, so that should be fine. (Obviously, my word means less than a profiler. If you see a huge spike in game thread usage when you switch weapons… then reconsider.)

There are ways you could make it more generalized, but, I mean, it ultimately only needs to do what your game requires, so even that’s meh.

You might have a problem if CurrentWeapon is neither PrimaryWeapon, nullptr, or SecondaryWeapon (which could happen if you change the primary or secondary weapon at runtime). You will softlock your ability to change weapons. In this case, since you seem to want your fail state to be secondary weapon.


void ALearnProjectCharacter::ChangeWeapon() {
	if (CurrentWeapon == PrimaryWeapon || CurrentWeapon == nullptr) {
		CurrentWeapon = SecondaryWeapon;
		PrimaryWeapon->SetActorHiddenInGame(true);
		SecondaryWeapon->SetActorHiddenInGame(false);
		UE_LOG(LogTemp, Warning, TEXT("SecondaryEquipped"))
	}
	else if(CurrentWeapon == SecondaryWeapon) {
		CurrentWeapon = PrimaryWeapon;
		PrimaryWeapon->SetActorHiddenInGame(false);
		SecondaryWeapon->SetActorHiddenInGame(true);
		UE_LOG(LogTemp, Warning, TEXT("PrimaryEquipped"))
	}
}

Should turn into


void ALearnProjectCharacter::ChangeWeapon() {
	if (!PrimaryWeapon || !SecondaryWeapon) { //Always good to check your pointers
		UE_LOG(LogTemp, Error, TEXT("Cannot Access Primary or Secondary Weapon"));
		return;
	}

	if (CurrentWeapon == SecondaryWeapon) {
		CurrentWeapon = PrimaryWeapon;
		PrimaryWeapon->SetActorHiddenInGame(false);
		SecondaryWeapon->SetActorHiddenInGame(true);
		UE_LOG(LogTemp, Warning, TEXT("Primary Equipped"));
	}
	else {
		CurrentWeapon = SecondaryWeapon;
		PrimaryWeapon->SetActorHiddenInGame(true);
		SecondaryWeapon->SetActorHiddenInGame(false);
		UE_LOG(LogTemp, Warning, TEXT("Secondary Equipped"));
	}
}

Alternatively, you could just have a boolean “bUsingPrimaryWeapon” or something. When you switch weapons, set bUsingPrimaryWeapon = !bUsingPrimaryWeapon.

Also, you might want to have these things be associated with animations. I’m not sure what type of game you’re making, though.

3

I dont know how good you are at c++ but i have my weapon base class and all my guns are child actors of it. Do you think it would be good to have 3 weapon base types like 1 for single shots 1 for burst fire and 1 for auto guns or should i just keep one weapon base because if i want a auto rifle and then the secondary is single shot the way im thinking of doing it will make them both automatic.

4

If you don’t know your mechanics yet, then I’d probably add all functionality into individual test weapons.

Once you know what’s fun, then you can define generalized classes that allow you to re-use code, and develop an actual hierarchy from there (should one make sense for your game).