search

Source-Python-Dev-Team / Source.Python

This plugin aims to use boost::python and create an easily accessible wrapper around the Source Engine API for scripter use.
http://forums.sourcepython.com
GNU General Public License v3.0
164 stars 32 forks source link

Improved WeaponClass, WeaponID, WeaponType, and WeaponSlot for CS:GO/CS:S. #478

Open CookStar opened 1 year ago

CookStar commented 1 year ago

Some WeaponID and WeaponType names were changed to match the names used internally in CS:GO/CS:S.

This will allow you to get the ID by weapon class name or pass an argument with WeaponType.name to an in-game function that takes the name of the WeaponType.

CookStar commented 1 year ago

Why is #459 not merged?

jordanbriere commented 1 year ago

I have not looked much into it but at first glance, I'm wondering if there is any specific reason for triplicating the data?

CookStar commented 1 year ago

What exactly are you referring to? I added WeaponID to csgo.ini, cstrike.ini first because it is easier to look up, and also because there is no item_definition_index in cstrike.

jordanbriere commented 1 year ago

Wouldn't it be better for item_definition_index to be removed completely, and for the enumerations to be referenced by name rather than value?

For example:

[[awp]]
    id = AWP
    type = SNIPERRIFLE
    slot = PRIMARY
    maxammo = 30
    ammoprop = 6
    clip = 10
    cost = 4750
    tags = "all,primary,rifle,sniper"
CookStar commented 1 year ago

Wouldn't it be better for item_definition_index to be removed completely,

Are you saying that item_definition_index should be deprecated? item_definition_index already used in SP...

and for the enumerations to be referenced by name rather than value?

Only CS:GO/CS:S are going to be handled like that? It seems that games other than TF2 don't have ID, Type, Slot constants in the first place.

jordanbriere commented 1 year ago

Are you saying that item_definition_index should be deprecated? item_definition_index already used in SP...

We can keep the property itself for backward compatibility, but I don't think duplicating the value for each section in the data file is necessary when both can point to the id field.

Only CS:GO/CS:S are going to be handled like that? It seems that games other than TF2 don't have ID, Type, Slot constants in the first place.

I think they all do, but we simply don't have data for them. Perhaps a simple getattr with a default could do the trick. In my opinion, it would be overall cleaner to map them by name where we can rather than hard-coding the same value multiple times.

CookStar commented 1 year ago

Only CS:GO/CS:S are going to be handled like that? It seems that games other than TF2 don't have ID, Type, Slot constants in the first place.

I think they all do, but we simply don't have data for them. Perhaps a simple getattr with a default could do the trick. In my opinion, it would be overall cleaner to map them by name where we can rather than hard-coding the same value multiple times.

Agreed.

Are you saying that item_definition_index should be deprecated? item_definition_index already used in SP...

We can keep the property itself for backward compatibility, but I don't think duplicating the value for each section in the data file is necessary when both can point to the id field.

The item_definition_index is used in CS:GO to sort out duplicate weapon classes, in CS:GO the item_definition_index and WeaponID are identical, but in TF2 there seems to be no relationship between the item_definition_index and WeaponID as far as I looked in items_game.txt. If TF2 doesn't have the problem of duplicate weapon classes, or if we don't plan to specifically reference item_definition_index in the future, we could reference it from WeaponID only. Is that all right then?

CookStar commented 1 year ago

All changes have been completed.

jordanbriere commented 1 year ago

The item_definition_index is used in CS:GO to sort out duplicate weapon classes, in CS:GO the item_definition_index and WeaponID are identical, but in TF2 there seems to be no relationship between the item_definition_index and WeaponID as far as I looked in items_game.txt. If TF2 doesn't have the problem of duplicate weapon classes, or if we don't plan to specifically reference item_definition_index in the future, we could reference it from WeaponID only. Is that all right then?

I guess that is something that can be addressed if data for this game is ever added.

All changes have been completed.

../instance.py#192 will produce a NameError. Other than that; LGTM.

CookStar commented 1 year ago

/instance.py#192 will produce a NameError. Other than that; LGTM.

I guess I didn't test with data that included digits, my bad.