Fixed error when trying to drop ammo for weapons without AmmoEnt

[MrXonte]

When trying to drop ammo on a weapon with no AmmoEnt (aka AmmoEnt = ""), an error appears in the console from trying to create a non-existent entity.

[MrXonte]

i just noticed that AmmoEnt isnt default in the base anyway, but it seems quite a few addons do add it with = "". Guess the addons should update instead of TTT2 then

[TimGoll]

Uhm, am I missing something or is your fix an actual valid fix? .AmmoEnt is indeed used in TTT2 (and in vanilla TTT as well):

https://github.com/TTT-2/TTT2/blob/73b4723052b4763d4506501106a8660cd5d36526/gamemodes/terrortown/entities/weapons/weapon_zm_mac10.lua#L37

https://github.com/TTT-2/TTT2/blob/73b4723052b4763d4506501106a8660cd5d36526/gamemodes/terrortown/entities/entities/base_ammo_ttt.lua#L96

[MrXonte]

Yea as per my comment i noticed that the issue is within addons, not TTT/TTT2 itself. A few addons use AmmoEnt = "" instead of just not setting AmmoEnt in SWEPS. If its not set the default code works perfectly fine, (due to the nil check) but if an Addon sets it to "", then it causes Errors. Should be the addon creators issue, that's why I closed the pull request, my bad

[TimGoll]

Yea as per my comment i noticed that the issue is within addons, not TTT/TTT2 itself. A few addons use AmmoEnt = "" instead of just not setting AmmoEnt in SWEPS. If its not set the default code works perfectly fine, (due to the nil check) but if an Addon sets it to "", then it causes Errors. Should be the addon creators issue, that's why I closed the pull request, my bad

Understood. But adding a failsafe check to TTT2 is perfectly reasonable in my opinion, even though the devs should set it to nil instead of "". Keep in mind that TTT2 is still actively maintained, while other addons are not.

Imho your fix sounds like a good addition. @Histalek should voice his opinion as well I think

[Histalek]

Could you send us an example of such an error?

As far as i can see we already check for invalid entity classes in this case, so i'm confused on how this could possibly lead to an error

[MrXonte]

Here i tried to drop ammo with a SWEP that has SWEP.AmmoEnt = "". Its not a "loud" error that gets reported in the problems section, but its one of the red "silent" errors. This error also happens when using random weapon spawns with matching ammo

[Histalek]

Ok i think i understand now.

The Attempted to create unknown entity type <the entity type in question>! is caused by the call to ents.Create [1] here [2]. We already check if the returned result is valid and prevent this becoming a lua error, but the console message is directly caused by the function call. Sadly i don't know of a way to check beforehand if an entity class exists and therefore we cannot guard against this.

So yeah this is a problem best solved on the individual swep addon side.

However if "" would be an extremely widely used ammoEnt i would be fine with the PR to prevent this specific case

[1] https://wiki.facepunch.com/gmod/ents.Create

[2] https://github.com/TTT-2/TTT2/blob/d6c1627fc093b1388ebd0426fc8638c562c601e5/gamemodes/terrortown/gamemode/server/sv_player_ext.lua#L1454

[TimGoll]

I agree with @Histalek and this is also how I understood it. Although I did not spend as much time to understand the real reason behind all this. I'll reopen this PR. If you, @MrXonte are against it, feel free to say so. Thanks for raising this issue!

[Histalek]

I've merged the check into our existing check and wrote a reasoning comment so we don't forget

Thanks again @MrXonte for finding and fixing this!