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

minor: triangle_drawing: added template function for drawing trigger polygons #535

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SmileyAG
Copy link
Collaborator

For drawing trigger polygons on pre and post 25th anniversary versions.

@YaLTeR
Copy link
Owner

YaLTeR commented Jul 27, 2024

Wait so shouldn't there be two different invocations of the template then? How would it look?

I'm asking because I'm not sure this cast should be done this exact way, but I need to see both invocations to tell for sure.

@SmileyAG
Copy link
Collaborator Author

SmileyAG commented Jul 31, 2024

but I need to see both invocations to tell for sure.

if (hw.is_hl25)
  DrawPolygons<msurface_25th_anniversary_hw_t*>(pTriAPI, model, i);
else
  DrawPolygons<msurface_t*>(pTriAPI, model, i);

Compilers will simply create two functions in binary, and in the function where triggers are drawn, it will add CALL instructions to them (E8 ?? ?? ?? ??)
Inside the function where polygons are drawn, there is a cast to the required data type in the form: (T)model->surfaces, so there are clearly no reasons here that point to UB, if this suddenly worries you.

But if your question actually was directed at why I didn’t do it initially, well it was because there is no HL25 support as such yet, although I can comment this piece of code in advance for later uncomment when it comes to it.

@YaLTeR
Copy link
Owner

YaLTeR commented Jul 31, 2024

Then if I understand correctly, that cast inside should be a reinterpret_cast<T> instead of (T) to prevent some of potential misuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants