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

Switch vector and angle types to use userdata types #2399

Merged
merged 3 commits into from
Sep 9, 2022

Conversation

Vurv78
Copy link
Member

@Vurv78 Vurv78 commented Sep 2, 2022

The E2 vector and angle types supported both the Vector and Angle gmod userdata types, as well as tables with three numbers in them. Since it had no idea which it was dealing with, it instead treated all of them as tables with three numbers, requiring constant calls to the vector/angle constructors for use in actual glua functions. This converts the e2 vector and angle types to only support gmod userdata (rather than also supporting tables with three numbers).

This should boost performance a good bit especially for physics/angle heavy E2s (and I still will need to make another PR to replace fixDefault to use a function, for further perf boosts)

⚠️ This breaks backwards compatibility with any E2 extensions giving tables to E2. Doing so will most likely cause lua errors now
Because of this, maybe we want to reach out to common extension creators to update their extensions to use the userdata instead. (Although I checked @sirpapate's extensions, they don't seem to return vector or angle, so should be fine.)

Fixes #2218

Constantly calling the vector and angle constructors is pretty bad for performance (especially needlessly as angle and vector were doing). This converts the e2 vector and angle types to only support gmod userdata (rather than also supporting tables with three numbers)
@bigdogmat
Copy link
Member

Besides the extension compact which will never be fixable, this also breaks anything that relies on using doubles as both vector and angle use floats for their numbers.

Should be all of them. Scanned over e2function vector|angle
Vurv78 added a commit to Vurv78/wire-extras that referenced this pull request Sep 6, 2022
**DO NOT MERGE THIS UNTIL wiremod/wire#2399 IS MERGED.**

This PR converts functions that return a table with three numbers to proper Vector/Angle userdata, since that is what vector and angle types will now use in E2 for efficiency.

I'm sweeping through found instances of this on github and making these PRs to make sure the amount of extensions broken by this is minimal.
Vurv78 added a commit to Vurv78/e2_vgui_core that referenced this pull request Sep 6, 2022
**DO NOT MERGE THIS UNTIL wiremod/wire#2399 IS MERGED.**

This PR converts functions that return a table with three numbers to proper Vector/Angle userdata, since that is what vector and angle types will now use in E2 for efficiency.

I'm sweeping through found instances of this on github and making these PRs to make sure the amount of extensions broken by this is minimal.
AlexALX added a commit to RafaelDeJongh/cap that referenced this pull request Sep 6, 2022
MattJeanes added a commit to MattJeanes/PewPew that referenced this pull request Sep 6, 2022
MattJeanes added a commit to MattJeanes/TARDIS that referenced this pull request Sep 6, 2022
MattJeanes added a commit to MattJeanes/TARDIS-Legacy that referenced this pull request Sep 6, 2022
dvdvideo1234 added a commit to dvdvideo1234/ControlSystemsE2 that referenced this pull request Sep 7, 2022
dvdvideo1234 added a commit to dvdvideo1234/ControlSystemsE2 that referenced this pull request Sep 7, 2022
dvdvideo1234 added a commit to dvdvideo1234/E2PistonTiming that referenced this pull request Sep 7, 2022
Should be faster than unpack
@Vurv78
Copy link
Member Author

Vurv78 commented Sep 7, 2022

Will merge tomorrow if there's no issues @dvdvideo1234 @Linus045 @HyvelEurope

dvdvideo1234 added a commit to dvdvideo1234/LaserSTool that referenced this pull request Sep 8, 2022
dvdvideo1234 added a commit to dvdvideo1234/TrackAssemblyTool that referenced this pull request Sep 8, 2022
@Vurv78 Vurv78 merged commit cc90f68 into wiremod:master Sep 9, 2022
E2 Overhaul automation moved this from Doing to Done Sep 9, 2022
Vurv78 added a commit to wiremod/wire-extras that referenced this pull request Sep 9, 2022
**DO NOT MERGE THIS UNTIL wiremod/wire#2399 IS MERGED.**

This PR converts functions that return a table with three numbers to proper Vector/Angle userdata, since that is what vector and angle types will now use in E2 for efficiency.

I'm sweeping through found instances of this on github and making these PRs to make sure the amount of extensions broken by this is minimal.
@thegrb93
Copy link
Contributor

thegrb93 commented Sep 9, 2022

Did this actually help performance? Starfall found this way of doing things to be much slower.

@Vurv78
Copy link
Member Author

Vurv78 commented Sep 9, 2022

wdym? starfall can't do this, since you provide stuff like debug.getinfo and getfenv to users (without compile time checks) there needs to be another level of indirection no matter what

I haven't benchmarked, but if it is slower, it's likely that some functions (especially from external extensions) aren't ported over yet and still support the old types, plus I haven't refactored out fixDefault which has a type call and has to copy/clone the type..

function E2Lib.fixDefault(var)
local t = type(var)
return t == "table" and table_Copy(var)
or t == "Vector" and Vector(var)
or t == "Angle" and Angle(var)
or var
end

Hopefully it can be changed to use a compile time lookup with types

@Vurv78 Vurv78 deleted the userdata-vectors branch September 9, 2022 22:58
@thegrb93
Copy link
Contributor

thegrb93 commented Sep 9, 2022

No, it's slower because creating Vector and Angle objects and using their metamethods is much slower than doing it in native lua. Starfall used to use the glua objects until we ported all the math to lua.

@Vurv78
Copy link
Member Author

Vurv78 commented Sep 9, 2022

Either way this is more oriented toward being able to use vector and angle properly as their glua types in physics E2s.

Creating a userdata and using that throughout the whole variable's lifetime is much better than constant calls to create Vector/Angle for each function call onto it. It's the same as removing Color() call pollution in draw hooks, to me.

If you just want a simple data structure without calling into glua types, you could use vector2/vector4/array/table, but 90% of the time you should be using vector/angle for physics / using in glua function calls. Although, E2 not having a color type forces you to use vector for data sometimes :p

@Vurv78
Copy link
Member Author

Vurv78 commented Sep 9, 2022

Another big shame is that glua and e2's vector apis are opposite, glua has mostly self-modifying operations, e2 has only copying-operations, so there are copies made in a good amount of cases anyway.

@dvdvideo1234
Copy link
Contributor

For now I think I'll keep two separate branches. One with the V/A mod and one without. I see the workshop is running this revision https://steamcommunity.com/linkfilter/?url=https://github.com/wiremod/wire/commit/7ed23af5b8fd800cb7e520bf7cfba4b813bfc1ae and the git HEAD is way ahead of it. Then in a way WORKSHOP users can install my master branch ( as it is right now ) and GIT users can install branch wire-pull-2399

dvdvideo1234 added a commit to dvdvideo1234/TrackAssemblyTool that referenced this pull request Oct 3, 2022
dvdvideo1234 added a commit to dvdvideo1234/E2PistonTiming that referenced this pull request Oct 3, 2022
dvdvideo1234 added a commit to dvdvideo1234/ControlSystemsE2 that referenced this pull request Oct 3, 2022
dvdvideo1234 added a commit to dvdvideo1234/LaserSTool that referenced this pull request Oct 3, 2022
@Vurv78 Vurv78 mentioned this pull request Apr 24, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Refactor E2 to use Vector() & Angle() constructors/userdata
4 participants