maximegmd / CyberEngineTweaks

Cyberpunk 2077 tweaks, hacks and scripting framework
MIT License
4.48k stars 285 forks source link

Add BasicTypes overload operators #916

Open poirierlouis opened 7 months ago

poirierlouis commented 7 months ago

Add overload operators for Vector3 and Vector4.

2024-01-20_CET_basictypes-overload-operators

WSSDude commented 7 months ago

I also think that the ovetloads should be added to RED4ext.SDK and used from there, wouldnt add more specializations into CET when the base is that, just makes things more cumbersome to maintain.

maximegmd commented 7 months ago

Looks good to me, I don't see how this could be made more performant, the compiler will do the SIMD optimization and calling the game function has a cost.

WSSDude commented 7 months ago

Unsure if this will get much SIMD optimization and unsure what is the cost of building the new object... Considering this was meant to shadow existing game functions anyway, I at least think it would be better to reuse them. They may have optimizations or specific checks we miss etc. (probably not, but you never know until you look at it, which I didnt at least...)

I'll pull my review if you want to merge it but dont find this particularly good personally, reimplementing things and potentially changing behavior...

Also think this should be part of RED4ext but that was another point, not performance-related.

WSSDude commented 7 months ago

Didnt mean to send it... Wanted to add that I would at least change the comparison operators to take into account some form of epsilon if nothing else. Same for other comparisons, is not the best to compare floating points directly like this.

WSSDude commented 7 months ago

Nah, ended up editing my comments anyway to add few points... Too late to apologise for multiple messages...

Dismissed my review, do what you want based on my comments I guess.

poirierlouis commented 7 months ago

About performance, I might write a tiny benchmark to compare time of execution between native methods and Lua implementation. See if there is a significant difference and, I guess, avoid any assumption on this matter. Maybe even detect different results...

About epsilon comparison, I was wondering if it should be used. Currently, it is not used with operator ==. But as you point out, it might be within native implementation. Hence, it might be better to rely on native implementation only.

About RED4ext.SDK implementation, if it can be directly implemented there and cascade to Lua without much effort, well yes. The though didn't cross my mind when talking about this missing "feature" in Lua. I'm wondering, currently files for Vector3 and Vector4 are generated. Would it be possible to write operators there without messing the code when generating them?

poirierlouis commented 7 months ago

Here a tiny-benchmark.txt you can rename with .lua extension and try yourself.

First test is most likely too simple regarding floating point precision... However difference between overload operators and call to native functions speak for itself IMO (loop of 100 000 iterations). Of course it is even worse when you get and call global function without storing a reference before running computation. So I didn't bother to let it in attachment. Elapsed time here is in seconds:

2024-01-21_CET_basictypes-overload-operators-tiny-benchmark

Feel free to improve this benchmark if it is too simple to be representative or point out any mistakes.

Edit: I had concerns order of execution between overload/native functions might have some kind of impact. Tried running native then overload, same results.

poirierlouis commented 7 months ago

See discussion on RED4ext.SDK.

poirierlouis commented 1 month ago

I'll update this PR after this is done https://github.com/WopsS/RED4ext.SDK/pull/140.