Skip to content

Exposing detailed knife details #431

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.

Sign up for GitHub

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

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

Exposing detailed knife details #431

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5c94794
added knife types
Falderebet Oct 5, 2023
94767d6
added test coverage for mapping of knives.
Falderebet Oct 5, 2023
f796928
linting
Falderebet Oct 5, 2023
25f3d76
Merge branch 'markus-wa:master' into exposing-detailed-wep-data
Falderebet Oct 23, 2023
15daf5e
Merge branch 'master' into exposing-detailed-wep-data
Falderebet Oct 23, 2023
5fb46b3
added RawEquipment to equipment struct
Falderebet Oct 23, 2023
2bd6010
added handling of rawequipment when binding weapon
Falderebet Oct 23, 2023
1a4fcdb
Merge branch 'exposing-detailed-wep-data' of https://github.com/Falde…
Falderebet Oct 23, 2023
3505ba3
Merge branch 'markus-wa:master' into exposing-detailed-wep-data
Falderebet Oct 23, 2023
85fcd76
fixed eq tests
Falderebet Oct 23, 2023
d71becb
Merge branch 'exposing-detailed-wep-data' of https://github.com/Falde…
Falderebet Oct 23, 2023
3256112
Merge branch 'markus-wa:master' into exposing-detailed-wep-data
Falderebet Nov 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 81 additions & 26 deletions pkg/demoinfocs/common/equipment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,25 @@ const (
EqWrench EquipmentType = 419
EqSnowball EquipmentType = 420
EqBumpMine EquipmentType = 421
EqKnifeBayonet EquipmentType = 422
EqKnifeCSS EquipmentType = 423
EqKnifeFlip EquipmentType = 424
EqKnifeGut EquipmentType = 425
EqKnifeKarambit EquipmentType = 426
EqKnifeM9Bayonet EquipmentType = 427
EqKnifeTactical EquipmentType = 428
EqKnifeFalchion EquipmentType = 429
EqKnifeSurvivalBowie EquipmentType = 430
EqKnifeButterfly EquipmentType = 431
EqKnifePush EquipmentType = 432
EqKnifeCord EquipmentType = 433
EqKnifeCanis EquipmentType = 434
EqKnifeUrsus EquipmentType = 435
EqKnifeGypsyJackknife EquipmentType = 436
EqKnifeOutdoor EquipmentType = 437
EqKnifeStiletto EquipmentType = 438
EqKnifeWidowmaker EquipmentType = 439
EqKnifeSkeleton EquipmentType = 440
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the RawEquipment part looks good now - but this part will need to be removed as it's not backwards compatible as mentioned in the other thread.

So all we should do for now is expose RawEquipment.

Maybe we can have a separate type KnifeType and constants KnifeTypeXYZ with a map KnifeTypes map[string]KnifeType which includes all known knife types to still provide some kind of mapping.

The good thing about this is that if a new knife comes out the users can just add it to the map themselves,. as the map will be mutable.

I think this would be the best middleground


// Grenades

Expand Down Expand Up @@ -204,6 +223,28 @@ func initEqNameToWeapon() {
eqNameToWeapon["vesthelm"] = EqHelmet
eqNameToWeapon["defuser"] = EqDefuseKit

eqNameToWeapon["knife"] = EqKnife
eqNameToWeapon["bayonet"] = EqKnifeBayonet
eqNameToWeapon["knife_bayonet"] = EqKnifeBayonet
eqNameToWeapon["knife_css"] = EqKnifeCSS
eqNameToWeapon["knife_flip"] = EqKnifeFlip
eqNameToWeapon["knife_gut"] = EqKnifeGut
eqNameToWeapon["knife_karambit"] = EqKnifeKarambit
eqNameToWeapon["knife_m9_bayonet"] = EqKnifeM9Bayonet
eqNameToWeapon["knife_tactical"] = EqKnifeTactical
eqNameToWeapon["knife_falchion"] = EqKnifeFalchion
eqNameToWeapon["knife_survival_bowie"] = EqKnifeSurvivalBowie
eqNameToWeapon["knife_butterfly"] = EqKnifeButterfly
eqNameToWeapon["knife_push"] = EqKnifePush
eqNameToWeapon["knife_cord"] = EqKnifeCord
eqNameToWeapon["knife_canis"] = EqKnifeCanis
eqNameToWeapon["knife_ursus"] = EqKnifeUrsus
eqNameToWeapon["knife_gypsy_jackknife"] = EqKnifeGypsyJackknife
eqNameToWeapon["knife_outdoor"] = EqKnifeOutdoor
eqNameToWeapon["knife_stiletto"] = EqKnifeStiletto
eqNameToWeapon["knife_widowmaker"] = EqKnifeWidowmaker
eqNameToWeapon["knife_skeleton"] = EqKnifeSkeleton
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, would need to be moved to the new map I wrote about


// These don't exist and / or used to crash the game with the give command
eqNameToWeapon["scar17"] = EqUnknown
eqNameToWeapon["sensorgrenade"] = EqUnknown
Expand Down Expand Up @@ -267,6 +308,25 @@ func initEqElementToName() {
eqElementToName[EqDefuseKit] = "Defuse Kit"
eqElementToName[EqKnife] = "Knife"
eqElementToName[EqUnknown] = "UNKNOWN"
eqElementToName[EqKnifeBayonet] = "Bayonet"
eqElementToName[EqKnifeCSS] = "CSS Knife"
eqElementToName[EqKnifeFlip] = "Flip Knife"
eqElementToName[EqKnifeGut] = "Gut Knife"
eqElementToName[EqKnifeKarambit] = "Karambit"
eqElementToName[EqKnifeM9Bayonet] = "M9 Bayonet"
eqElementToName[EqKnifeTactical] = "Tactical Knife"
eqElementToName[EqKnifeFalchion] = "Falchion Knife"
eqElementToName[EqKnifeSurvivalBowie] = "Survival Bowie Knife"
eqElementToName[EqKnifeButterfly] = "Butterfly Knife"
eqElementToName[EqKnifePush] = "Push Knife"
eqElementToName[EqKnifeCord] = "Cord Knife"
eqElementToName[EqKnifeCanis] = "Canis Knife"
eqElementToName[EqKnifeUrsus] = "Ursus Knife"
eqElementToName[EqKnifeGypsyJackknife] = "Gypsy Jackknife"
eqElementToName[EqKnifeOutdoor] = "Outdoor Knife"
eqElementToName[EqKnifeStiletto] = "Stiletto Knife"
eqElementToName[EqKnifeWidowmaker] = "Widowmaker Knife"
eqElementToName[EqKnifeSkeleton] = "Skeleton Knife"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would have to be in a new map knifeTypeToName - don't forget to implement the String() method on KnifeType

}

const weaponPrefix = "weapon_"
Expand All @@ -276,13 +336,8 @@ const weaponPrefix = "weapon_"
func MapEquipment(eqName string) EquipmentType {
eqName = strings.TrimPrefix(eqName, weaponPrefix)

var wep EquipmentType
if strings.Contains(eqName, "knife") || strings.Contains(eqName, "bayonet") {
wep = EqKnife
} else {
// If the eqName isn't known it will be EqUnknown as that is the default value for EquipmentType
wep = eqNameToWeapon[eqName]
}
// If the eqName isn't known it will be EqUnknown as that is the default value for EquipmentType
wep := eqNameToWeapon[eqName]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this doesn't really scale well for when they add new knifes - we would need to update the parser every time. and worse, our consumers would need to upgrade, otherwise they will get EqUnknown for the new knife, which would be really hard to debug.

I suggest that we instead expose the raw eqName somewhere upstream in the call chain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point, I will take a look at it!

Copy link
Contributor Author

@Falderebet Falderebet Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**Edit - I found the attribute "m_sWeaponName". Is that what you (@markus-wa) meant regarding exposing the raw eqName?

If so disregard the rest of the comment.

***Edit2 - I digged a bit deeper regarding the "m_sWeaponName" property, and it seems like it is not possible to get any relevant data from it. I have not found any other properties that holds weapon name data. Let me know if you know of any.

I don't think there is a good way to handle valve adding new knives. However, I think there are some options:

  1. Exposing the weapon "code"/itemindex - eg. we give the consumers the possibility to see what code the EqUnkown has.

    • Pros:
      • This technically means that consumers can still handle new knives even before the parser is updated.
    • Cons:
      • It will still to a degree be hard for consumers to debug, as they only get a weapon code and nothing "human readable".
      • Parser will still need to be updated to handle new knives with EqTypes and so forth.

  2. Exposing a more raw weapon string.

    • Pros:
      • Makes it more human readable for consumers to debug.
    • Cons:
      • The parser needs to be updated before consumers can add new knives.

After writing this out I find myself gravitating more towards option 1. As the raw weapon string is not something coming from the engine but something we attach from the item index.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also why not keeping strings.Contains(eqName, "knife") as a fallback when wep is unknown?
i think it's better to have weapon_knife instead of "Unknown" when we know it's a knife

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also why not keeping strings.Contains(eqName, "knife") as a fallback when wep is unknown? i think it's better to have weapon_knife instead of "Unknown" when we know it's a knife

My thought was that the parser defines what a knife "is". Therefore it would be an error in the parser if the EqName did not map to an EqType.

I can add it back, I guess it is nice to have if new knives are added and the eqname/eqtype map is not updated.

Copy link
Owner

@markus-wa markus-wa Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually - I don't think we can add new EqKnifeXYZ types anyway as it's not backwards compatible and would lead to confusion (e.g. if the user wants to check if a kill was a knife kill, using EqKnife is more straight forward than checking all cases.

So we should expose this as additional data, not as replacement.

So we'll need another way of solving this - not yet sure how exactly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the idea of adding a field to the equipment struct with additional detailes about the weapon/equipment?

Let me know if there is something I can do. :)

Copy link
Owner

@markus-wa markus-wa Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good! Maybe something similar to what we already have with Equipment.OriginalString but for this new string?


return wep
}
Expand Down Expand Up @@ -538,23 +593,23 @@ var EquipmentIndexMapping = map[uint64]EquipmentType{
83: EqHE, // weapon_frag_grenade
84: EqSnowball, // weapon_snowball
85: EqBumpMine, // weapon_bumpmine
500: EqKnife, // weapon_bayonet
503: EqKnife, // weapon_knife_css
505: EqKnife, // weapon_knife_flip
506: EqKnife, // weapon_knife_gut
507: EqKnife, // weapon_knife_karambit
508: EqKnife, // weapon_knife_m9_bayonet
509: EqKnife, // weapon_knife_tactical
512: EqKnife, // weapon_knife_falchion
514: EqKnife, // weapon_knife_survival_bowie
515: EqKnife, // weapon_knife_butterfly
516: EqKnife, // weapon_knife_push
517: EqKnife, // weapon_knife_cord
518: EqKnife, // weapon_knife_canis
519: EqKnife, // weapon_knife_ursus
520: EqKnife, // weapon_knife_gypsy_jackknife
521: EqKnife, // weapon_knife_outdoor
522: EqKnife, // weapon_knife_stiletto
523: EqKnife, // weapon_knife_widowmaker
525: EqKnife, // weapon_knife_skeleton
500: EqKnifeBayonet, // weapon_bayonet
503: EqKnifeCSS, // weapon_knife_css
505: EqKnifeFlip, // weapon_knife_flip
506: EqKnifeGut, // weapon_knife_gut
507: EqKnifeKarambit, // weapon_knife_karambit
508: EqKnifeM9Bayonet, // weapon_knife_m9_bayonet
509: EqKnifeTactical, // weapon_knife_tactical
512: EqKnifeFalchion, // weapon_knife_falchion
514: EqKnifeSurvivalBowie, // weapon_knife_survival_bowie
515: EqKnifeButterfly, // weapon_knife_butterfly
516: EqKnifePush, // weapon_knife_push
517: EqKnifeCord, // weapon_knife_cord
518: EqKnifeCanis, // weapon_knife_canis
519: EqKnifeUrsus, // weapon_knife_ursus
520: EqKnifeGypsyJackknife, // weapon_knife_gypsy_jackknife
521: EqKnifeOutdoor, // weapon_knife_outdoor
522: EqKnifeStiletto, // weapon_knife_stiletto
523: EqKnifeWidowmaker, // weapon_knife_widowmaker
525: EqKnifeSkeleton, // weapon_knife_skeleton
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these would need to be put in a new map as well KnifeTypeIndexMapping

}
26 changes: 24 additions & 2 deletions pkg/demoinfocs/common/equipment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,35 @@ func TestEquipmentElement_Name(t *testing.T) {
}

func TestMapEquipment(t *testing.T) {
assert.Equal(t, EqKnife, MapEquipment("weapon_bayonet"), "'weapon_bayonet' should be mapped to EqKnife")
assert.Equal(t, EqKnife, MapEquipment("weapon_knife_butterfly"), "'weapon_knife_butterfly' should be mapped to EqKnife")
assert.Equal(t, EqKnifeBayonet, MapEquipment("weapon_bayonet"), "'weapon_bayonet' should be mapped to EqKnifeBayonet")
assert.Equal(t, EqKnifeButterfly, MapEquipment("weapon_knife_butterfly"), "'weapon_knife_butterfly' should be mapped to EqKnifeButterfly")
assert.Equal(t, EqM4A4, MapEquipment("weapon_m4a1"), "'weapon_m4a1' should be mapped to EqM4A4") // This is correct, weapon_m4a1 == M4A4
assert.Equal(t, EqM4A1, MapEquipment("weapon_m4a1_silencer"), "'weapon_m4a1_silencer' should be mapped to EqM4A1")
assert.Equal(t, EqUnknown, MapEquipment("asdf"), "'asdf' should be mapped to EqUnknown")
}

func TestMapEquipmentKnives(t *testing.T) {
assert.Equal(t, EqKnifeBayonet, MapEquipment("weapon_knife_bayonet"), "'weapon_knife_bayonet' should be mapped to EqKnifeBayonet")
assert.Equal(t, EqKnifeCSS, MapEquipment("weapon_knife_css"), "'weapon_knife_css' should be mapped to EqKnifeCSS")
assert.Equal(t, EqKnifeFlip, MapEquipment("weapon_knife_flip"), "'weapon_knife_flip' should be mapped to EqKnifeFlip")
assert.Equal(t, EqKnifeGut, MapEquipment("weapon_knife_gut"), "'weapon_knife_gut' should be mapped to EqKnifeGut")
assert.Equal(t, EqKnifeKarambit, MapEquipment("weapon_knife_karambit"), "'weapon_knife_karambit' should be mapped to EqKnifeKarambit")
assert.Equal(t, EqKnifeM9Bayonet, MapEquipment("weapon_knife_m9_bayonet"), "'weapon_knife_m9_bayonet' should be mapped to EqKnifeM9Bayonet")
assert.Equal(t, EqKnifeTactical, MapEquipment("weapon_knife_tactical"), "'weapon_knife_tactical' should be mapped to EqKnifeTactical")
assert.Equal(t, EqKnifeFalchion, MapEquipment("weapon_knife_falchion"), "'weapon_knife_falchion' should be mapped to EqKnifeFalchion")
assert.Equal(t, EqKnifeSurvivalBowie, MapEquipment("weapon_knife_survival_bowie"), "'weapon_knife_survival_bowie' should be mapped to EqKnifeSurvivalBowie")
assert.Equal(t, EqKnifeButterfly, MapEquipment("weapon_knife_butterfly"), "'weapon_knife_butterfly' should be mapped to EqKnifeButterfly")
assert.Equal(t, EqKnifePush, MapEquipment("weapon_knife_push"), "'weapon_knife_push' should be mapped to EqKnifePush")
assert.Equal(t, EqKnifeCord, MapEquipment("weapon_knife_cord"), "'weapon_knife_cord' should be mapped to EqKnifeCord")
assert.Equal(t, EqKnifeCanis, MapEquipment("weapon_knife_canis"), "'weapon_knife_canis' should be mapped to EqKnifeCanis")
assert.Equal(t, EqKnifeUrsus, MapEquipment("weapon_knife_ursus"), "'weapon_knife_ursus' should be mapped to EqKnifeUrsus")
assert.Equal(t, EqKnifeGypsyJackknife, MapEquipment("weapon_knife_gypsy_jackknife"), "'weapon_knife_gypsy_jackknife' should be mapped to EqKnifeGypsyJackknife")
assert.Equal(t, EqKnifeOutdoor, MapEquipment("weapon_knife_outdoor"), "'weapon_knife_outdoor' should be mapped to EqKnifeOutdoor")
assert.Equal(t, EqKnifeStiletto, MapEquipment("weapon_knife_stiletto"), "'weapon_knife_stiletto' should be mapped to EqKnifeStiletto")
assert.Equal(t, EqKnifeWidowmaker, MapEquipment("weapon_knife_widowmaker"), "'weapon_knife_widowmaker' should be mapped to EqKnifeWidowmaker")
assert.Equal(t, EqKnifeSkeleton, MapEquipment("weapon_knife_skeleton"), "'weapon_knife_skeleton' should be mapped to EqKnifeSkeleton")
}

func TestEquipment_Class(t *testing.T) {
assert.Equal(t, EqClassUnknown, NewEquipment(EqUnknown).Class(), "EqUnknown should have the class EqClassUnknown")
assert.Equal(t, EqClassPistols, NewEquipment(EqP2000).Class(), "EqP2000 should have the class EqClassPistols")
Expand Down