Skip to content

Change int to any with Address Functions#1250

Merged
Headline merged 2 commits intoalliedmodders:masterfrom
Scags:addr-any
May 8, 2020
Merged

Change int to any with Address Functions#1250
Headline merged 2 commits intoalliedmodders:masterfrom
Scags:addr-any

Conversation

@Scags
Copy link
Contributor

@Scags Scags commented Apr 26, 2020

Since getting from and storing to addresses gives no regard to the type you throw at it, it makes sense to change these to any.

Would help with having to do some obnoxious tagging at the moment:

// Before
public void OnPluginStart()
{
	Address foo;
	float bar = 12.0;
	StoreToAddress(foo, view_as< int >(bar), NumberType_Int32);
	g_flFooBar = view_as< float >(LoadFromAddress(foo, NumberType_Int32));
}
// After
public void OnPluginStart()
{
	Address foo;
	float bar = 12.0;
	StoreToAddress(foo, bar, NumberType_Int32);
	g_flFooBar = LoadFromAddress(foo, NumberType_Int32);
}

@nosoop
Copy link
Contributor

nosoop commented May 7, 2020

There's one pitfall with this change on LoadFromAddress:

Address pFloat; // address of a float value
float flSomeMathResult = LoadFromAddress(pFloat, NumberType_Int32) + 1.0;

The addition operator will always treat the left operand as an int, in either case you'd have to explicitly reinterpret it as a float but the current documentation makes the integer behavior clear.

I think having a separate LoadFromAddressFloat and / or a compiler warning on implicit any conversion in general would help there.

@Scags
Copy link
Contributor Author

Scags commented May 7, 2020

Fair point, but there's two things here:

1: People who are even trying to do this kind of stuff would (should) be aware of how float operations work.
2: There are already SourceMod natives that perform the same action in the same manner (like GetArrayCell/ArrayList.Get).

I think compiler warnings on implicit any -> float are a bit overkill, though. On the other hand, a #pragma to warn against it on certain functions would be a cool feature.

@dvander
Copy link
Member

dvander commented May 7, 2020

I can't think of any reason not to do this, but it should be clarified that they're 32-bit loads if that's not done already.

@Scags
Copy link
Contributor Author

Scags commented May 7, 2020

Certainly. Added notes concerning bit size with the latest commit.

@Headline Headline merged commit 44615b7 into alliedmodders:master May 8, 2020
nosoop pushed a commit to nosoop/stocksoup that referenced this pull request Jul 19, 2020
@Scags
Make LoadStringFromAddress SM1.11 compatible (#5)
a61ac3c 
Github PR alliedmodders/sourcemod#1250 causes spcomp to error on `LoadStringFromAddress` due to not being able to assign `char[]` to `any[]` due to differing storage classes.

Reinterpreting the result as an integer clears up the error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dvander dvander dvander approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Scags @nosoop @dvander @Headline