x64 support via <virtual_address>

[Malifox]

Requires 1.13.0.7278 or greater. No breaking changes and I didn't find any compatibility issues. For 64-bit servers with plugins that rely on tf2attributes, as long as the plugin doesn't directly load address values(ex: LoadFromAddress()), then it should work as is without needing a recompile to include <virtual_address>.

Also included is a testing plugin to test all current natives with a single sm_test_tf2attributes command. I had a slight whitespace mishap when it came time break things into smaller commits that I'm hoping can be rebased out easily enough.

This solves #50

[FlaminSarge]

Would it be worth me removing all the trailing spaces and you rebasing so that they aren't a part of this PR?

[Malifox]

If you find that to be the simplest method and don't mind the loss of those _^evil whitespaces, I otherwise didn't mess up the formatting and that should work out. I didn't disable auto trim on save when I worked on this and I discovered the error of my ways after nearly everything was written. With quite a few changed lines needing be broken up into commits, the "I'll figure it out later" idea sounded good, at least at the time. Which of course left me with a bunch of whitespace diffs I don't know what to do with.

Other things of note, I left in the complete offsets layout for the used classes, which I found very useful in other places and aren't too likely to change at this point. Could be easily trimmed out if you find that it bloats things too much though.

[FlaminSarge]

load-bearing whitespace

I'll go ahead and strip all of those from master branch and see how close I can get to your diff having no whitespace changes, and then have you rebase on top of that; hopefully that won't be too painful.

[Malifox]

Something else to consider is that this can currently only be loaded on 1.13.0.7278+, the LoadAddressFromAddress() and StoreAddressToAddress() natives and public const Address PointerSize do not exist in the stable 1.12 branch, which complicates moving to this a bit actually.

[JagerPup]

Something else to consider is that this can currently only be loaded on 1.13.0.7278+, the LoadAddressFromAddress() and StoreAddressToAddress() natives and public const Address PointerSize do not exist in the stable 1.12 branch, which complicates moving to this a bit actually.

Any version of TF2 Attributes for 64-bit

[nosoop]

@FlaminSarge Feels like we've been sitting on this for too long; finally got around to the formatting nits. Anything else you want to comment on or should we just ship the thing?

Thinking we should set it up as a pre-release and maintain stable builds if 32-bit / 1.12 needs updates.

[Maxim766]

@Malifox <virtual_address> was removed since 1.13.7302 sm build. Any plans to remake this PR for newer builds of sm?

[FlaminSarge]

Currently trying to figure out the best way to deal with setting String attributes since the SM rework seems to have thrown a bit of a wrench into it.

[SageDarkwind]

I tried compiling it and got errors for "undefined symbol "PointerSize"".

[Malifox]

@Malifox <virtual_address> was removed since 1.13.7302 sm build. Any plans to remake this PR for newer builds of sm?

I happened to end up taking a break from SM shortly after making this PR and thus felt less eager to be an early adopter of a new address implementation this time, so I haven't looked at this until now. Initial compile attempt doesn't look too bad, but has 1 compile blocking issue. It seems StringMap and IntMap aren't yet compatible with int64?

Currently trying to figure out the best way to deal with setting String attributes since the SM rework seems to have thrown a bit of a wrench into it.

This is probably the main headache. With virtual addresses having been always 4 bytes, I simply continued abusing m_flValue for those. I recall thinking that was would end up being a problem in the future, but didn't really have any ideas on how that might be solved.

I think this really only applies to TF2Attrib_SetFromStringValue when the attribute is non-networked as well as TF2Attrib_UnsafeGetStringValue? I've never come across a real world example of a plugin setting non-networked values, so I'm not sure if anyone actually ever does that. For strings, I'd expect people would just want to read them with TF2Attrib_HookValueString, which is probably still fine.

[FlaminSarge]

It may be best for us to just drop support for setting String attributes via SetRuntimeValue, at least on x64.
Am actively working on this PR right now, wanted to take a stab at removing the giant if/else blocks for PointerSize.
SM actually merging something for x64 Addresses to mainline was a good motivation.

[Maxim766]

@Kenzzer Hello, could you help us, please? We need a bit more explanation how to deal with strings in new sm builds for setting up string attribute values

[Kenzzer]

Containers to store int64 are not planned anytime soon. Primary purpose of int64 was first and foremost to get address viable on x64. I strongly encourage to either rethink or completely discard anything that stores int64.

As for natives returning address/int64, this something we can fix. But if it's possible for you guys to do without then let's try to keep it that way. Ping me again if its a blocking point and I will make a native for int64 return in plugin natives.

[Malifox]

It may be best for us to just drop support for setting String attributes via SetRuntimeValue, at least on x64. Am actively working on this PR right now, wanted to take a stab at removing the giant if/else blocks for PointerSize. SM actually merging something for x64 Addresses to mainline was a good motivation.

Possibly, would need some probably clunky solution to make that work otherwise, already have 2 of those with the duplicated PrepSDKCalls to deal with the attribute union type.

Yeah, the wall of if/else blocks did stand out a bit. Wasn't sure how you would want that to look or if you even wanted to keep the full layout for completeness, so I just left as it was.

@Kenzzer Hello, could you help us, please? We need a bit more explanation how to deal with strings in new sm builds for setting up string attribute values

The setting strings issue only applies to attributes listed in items.txt with "attribute_type" "string", of which there are only 20 and trying to set those usually won't have the effect you think it would. For example, setting custom_name_attr won't change what clients see on a weapon's nametag due to attribute string values being non-networked from the server. In theory, "cosmetic taunt sound" would maybe be one of the very few where this really does something as the server doesn't need to network its value, just EmitSound(), but I've never tested that one. TF2Attrib_SetFromStringValue("major move speed bonus", "3.0"); where "3.0" is passed as a string rather than a float would be unaffected since that is a networked attribute.

@Kenzzer
From the plugin sources that I can quickly search through, I see at least 2 plugins that rely on tf2attributes returning an address from a native with the first heavily using TF2Attrib_GetByDefIndex:
https://forums.alliedmods.net/showthread.php?p=2766587
https://github.com/Phil25/RTD/blob/26aba4fc19b7390fd1e4a8b341bb25abfae4e068/scripting/rtd/perks/powerplay.sp#L135
Those particular use cases could probably be replaced with new, non address returning natives, but maybe not all possible use cases could be. The first needs additional work regardless, but RTD would work as is (just needing to be compiled after int64 was added).

As for storing Address/int64 in StringMap and such, I see 2 cases here where StringMap is used to save from needing to repeat 2 SDKCalls, the IntMap saves on 3. Storing those could be avoided, but would feel a bit unfortunate. However, I'm not sure if it could really be avoided for the AllocPooledString() related parts involved with TF2Attrib_HookValueString.

[FlaminSarge]

Yeah, the wall of if/else blocks did stand out a bit. Wasn't sure how you would want that to look or if you even wanted to keep the full layout for completeness, so I just left as it was.

Take a look at latest changes, it's still messy-looking but is much more resilient to when things shift around (if they're ever shifting around) and handles 32 and 64 cases in one go.

[Malifox]

Wasn't sure if something like that would work out with how x64 introduces more padding for alignment, so is pretty interesting to see that it did. Something that may potentially be a downside though is that if you or someone copy pasting these wants to uncomment a line to make use of, it wouldn't work unless you also uncomment and initialize all the ones prior. The advantage of having to worry less about those changing can be rather valuable though.

I sourced those offsets using Visual Studio's memory layout feature, so finding changed/new values is much easier now days without double the IDA fun (excluding closed source tf2 mods). Though, Microsoft never did figure out how copy paste from a table works, so I had to more manually make those and served as my only text-selectable and searchable version to refer to.