Proper way to strip static attributes #40
Conversation
The only way to maintain attributes such as min_viewmodel_offset is to not remove them in the first place. We have to manually nullify all the default attributes the weapon has.
|
I've tested this extensively ( not that much in CWX, but rather in other cases where static attributes would get stripped ). |
|
There's also this solution. I'm a bit iffy about the additional overhead of hooking items and the overhead. I've also experimented with similar attempts but by sendproxying the I think the "ideal" solution would be to flesh out the API more and add a post-create forward; that way a module can hook items and process them however they'd like, and changes can happen more rapidly outside of the codebase. |
scripting/cwx/item_config.sp
Outdated
| int staticAttribs[16]; | ||
| float staticAttribsValues[16]; | ||
|
|
||
| TF2Attrib_GetStaticAttribs(item.defindex, staticAttribs, staticAttribsValues); |
There was a problem hiding this comment.
TF2Econ_GetItemStaticAttributes instead? The game can technically store more than that.
There was a problem hiding this comment.
That's the original approach I used since at the time TF2Attrib_GetStaticAttribs didn't work for some reason. I don't think it should cause any issues since I don't think there is a weapon that has over 16 attributes (?)
There was a problem hiding this comment.
Yeah, that's probably fine. TF2Attrib_GetStaticAttribs returns the attribute count so it might be good to iterate up to that count as long as it's less than 16.
There was a problem hiding this comment.
Addressed this in the newest commit.
scripting/cwx/item_config.sp
Outdated
| } | ||
|
|
||
| // Probably overkill | ||
| if(staticAttribs[i] == 796 || staticAttribs[i] == 724 || staticAttribs[i] == 817 || staticAttribs[i] == 834 |
There was a problem hiding this comment.
If we can get a working implementation without hardcoded attributes, that'd be good. This kind of thing is brittle and not something I want to think about maintaining in the future.
I think the main ones would be paintkit_proto_def_index (834) and has team color paintkit (745); the other ones have empty description formats so I'd imagine they're not an issue.
There was a problem hiding this comment.
Scratch that; the attribute listing page lied to me. The other ones do have value_is_additive.
There was a problem hiding this comment.
So should I remove them? Although hardcoding some attributes might not be the prettiest thing to do ( which is fair ), I did it to skip some unnecessary cycles and avoid some things breaking ( e.g. 817 inspect_viewmodel_offset, I don't know if nullifying would actually break some "QoL" aspects of the weapons so I put them just in case ).
There was a problem hiding this comment.
I'd say remove them, but at the same time having other things be potentially broken isn't great. This was also one of the things I wasn't satisfied with when I was brainstorming the thing myself.
There was a problem hiding this comment.
That's fair. However, the amount of attributes is so small that it's negligible. It'd be nice to have an automatic way though.
There was a problem hiding this comment.
Addressed this in the newest commit.
scripting/cwx/item_config.sp
Outdated
| SetEntProp(itemEntity, Prop_Send, "m_bOnlyIterateItemViewAttributes", | ||
| !item.bKeepStaticAttributes); | ||
|
|
||
| if(!item.bKeepStaticAttributes) { |
There was a problem hiding this comment.
Might be time to refactor the config definition to support string values for keep_static_attrs? I think it's worth leaving the numeric options to the property.
Something like:
- Numeric values =
KeepStatic_FalseorKeepStatic_True; only togglem_bOnlyIterateItemViewAttributes - "nullify" =
KeepStatic_Nullify; run this code
There was a problem hiding this comment.
(Previous iterations claimed "2" was for nullifying; I don't have any issues with that approach either.)
There was a problem hiding this comment.
I haven't experienced any issues by using this method to strip static attributes but sure. I'd go with 0 / 1 / nullify.
There was a problem hiding this comment.
Addressed this in the newest commit.
scripting/cwx/item_config.sp
Outdated
| // Since we already know what we're working with and what we're looking for, we can manually handpick | ||
| // the most significative chars to check if they match. Eons faster than doing StrEqual or StrContains. | ||
|
|
||
| if(valueFormat[9] == 'a' && valueFormat[10] == 'd') { // value_is_additive & value_is_additive_percentage |
There was a problem hiding this comment.
Fairly sure this also breaks Flame Throwers — one of the frequent complaints (with CW3) is that after Jungle Inferno, not preserving attributes disabled the new FT params. Granted they'd still be broken with the current keep_static_attrs implementation.
It doesn't show up in my attribute listing page but they're all value_is_additive in items_game (oddly enough, IEconItems_440/GetSchemaOverview doesn't match here).
There was a problem hiding this comment.
(Previously thought about in #2.)
There was a problem hiding this comment.
Fair, I thought that it could break some niche weapons that had attributes that affected the functionality of the weapon. In my personal case, I didn't pay too much attention to it as it'd just be a matter of setting the attribute back to what it has to be in the config file of the weapon.
That's tricky. Would there even be a proper way to do it without hardcoding the attributes?
There was a problem hiding this comment.
With FT specifically, one could do a lookup to check the static attributes on the base item. The other base items (excluding spellbook / grappling hook / MvM canteen) only have weapon_stattrak_module_scale and kill eater score type * attributes. The FT does have extinguish restores health by default as well, though.
There was a problem hiding this comment.
There is not a proper way to retrieve the base item unfortunately ( without doing some ugly boilerplate, at least ). So honestly I'd just hardcode the attributes to skip since there is only a small amount anyway.
|
Appreciate the PR! I've considered something similar and was never satisfied with having to parse out |
…erate based on TF2Attrib_GetStaticAttribs
|
Yeah, I share the frustration with you. This is the only "proper" way I've found to tackle this issue without resorting to SDKCalls or DHooks. It does work, but it's not the prettiest. |
The only way ( that we currently know of ) is to nullify the attributes rather than stripping them entirely.