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

Change unnecessary int[]'s to any[] #1221

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

TotallyMehis
Copy link
Contributor

Nothing should stop you from using eg. a float array on these natives.

It gets kinda annoying to use view_as<int>()

@JoinedSenses
Copy link
Contributor

Has this been tested with enum structs?

@asherkin
Copy link
Member

Any thoughts on how this does/should behave when dataSize/size is not equal to 4 and a float array is passed?

Has this been tested with enum structs?

They don't have a stable in-memory representation so I don't think it really matters, but there shouldn't be any technical problems with these and them.

@TotallyMehis
Copy link
Contributor Author

Any thoughts on how this does/should behave when dataSize/size is not equal to 4 and a float array is passed?

In that case, yea, it would make sense to have a separate func for floats like GetEntDataArrayFloat that doesn't allow the data size to be changed.

I guess this PR depends on the trajectory and philosophy of SP. Personally, I want to give plugin authors all the power. Want to pass an enum struct or float array and do some weird magic with the data size? Go ahead.

@Headline Headline added the Feature Request user requested feature label May 8, 2020
@KyleSanderson
Copy link
Member

@asherkin I think this is cool. The natives are already pretty unsupported ({G,S}etEntData in the sense that they're completely unsafe - opening this up to what was intended (arbitrary sizes) will likely make things more clear.

With this being said, if we macro'd this that would be the way to make it more robust.

Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

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

Yeah, this LGTM

@asherkin asherkin merged commit 4e0ae0c into alliedmodders:master Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request user requested feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants