Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

CookStar
Copy link
Contributor

@CookStar CookStar commented Jun 6, 2023

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 added 4 commits June 6, 2023 11:31
Added WeaponType to CS:GO.
Changed WeaponClass.slot to return WeaponSlot.

Changed WeaponClass to be able to reference parent_class.

Added missing WeaponClass data for CS:GO.
@CookStar
Copy link
Contributor Author

CookStar commented Jun 6, 2023

Why is #459 not merged?

@jordanbriere
Copy link
Contributor

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
Copy link
Contributor Author

CookStar commented Jun 8, 2023

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
Copy link
Contributor

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
Copy link
Contributor Author

CookStar commented Jun 8, 2023

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
Copy link
Contributor

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
Copy link
Contributor Author

CookStar commented Jun 9, 2023

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 added 2 commits June 10, 2023 11:56
…nts' name.

Added sniperrifle, assaultrifle, submachinegun, mg tags to CS:GO/CS:S weapon data.

Added cant_create, cant_find, and parent tags to CS:GO weapon data.

Changed Entity.create, Entity.find, and Weapon.weapon_name in CS:GO to use new added tags with WeaponClassIter.
@CookStar
Copy link
Contributor Author

All changes have been completed.

@jordanbriere
Copy link
Contributor

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
Copy link
Contributor Author

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants