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

[FEATURE REQUEST] set_hudmessage add missing parameters #981

Merged
merged 4 commits into from
Aug 16, 2021
Merged

[FEATURE REQUEST] set_hudmessage add missing parameters #981

merged 4 commits into from
Aug 16, 2021

Conversation

francoromaniello
Copy link
Contributor

@francoromaniello francoromaniello commented Aug 16, 2021

Related to pull request #628 and #980 add set_hudmessage_ex with the missing params but for branch 1.9-dev

*
* @noreturn
*/
native set_hudmessage_ex(red = 200, green = 100, blue = 0, alpha = 0, red2 = 255, green2 = 255, blue2 = 250, alpha2 = 0, Float:x = -1.0, Float:y = 0.35, effects = 0, Float:fxtime = 6.0, Float:holdtime = 12.0, Float:fadeintime = 0.1, Float:fadeouttime = 0.2, channel = -1);
Copy link
Member

Choose a reason for hiding this comment

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

why is this new native needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set_hudmesage have alpha1, r2, g2, b2, alpha2 hardcoded, this native make the possibility of use all the params available for huds.

image
image

Copy link

Choose a reason for hiding this comment

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

Needed because we will have more flexibility on how hudmessages are displayed, so we can create more cool effects on them. (wondering why that value is hardcoded)

Copy link
Member

@dvander dvander Aug 16, 2021

Choose a reason for hiding this comment

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

You can extend the old native. Add parameters like "alpha1 = 255, color2[4] = {0, 0, 0, 0}" (or whatever the defaults should be for backward compatibility).

In the native, you can figure out the number of parameters passed by doing:

cell num_params = params[0] / sizeof(cell);

Then, if num_params >= 13 (the new count), you can safely extract the additional values. This will make the new signature work with old .amxx files as well as old scripts using the new include.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@francoromaniello francoromaniello changed the title [FEATURE REQUEST] set_hudmessage_ex [FEATURE REQUEST] set_hudmessage add miss params Aug 16, 2021
@francoromaniello francoromaniello changed the title [FEATURE REQUEST] set_hudmessage add miss params [FEATURE REQUEST] set_hudmessage add missing parameters Aug 16, 2021
@dvander
Copy link
Member

dvander commented Aug 16, 2021

Thanks! LGTM

@dvander dvander merged commit d1fc99f into alliedmodders:1.9-dev Aug 16, 2021
This was referenced Aug 16, 2021
@RauliTop
Copy link

@dvander
This should be added on 1.10-dev too

@rtxa
Copy link
Contributor

rtxa commented Aug 16, 2021

@dvander Glad they are merging this, but bad at the same time because I did a pull request for the same thing a long time ago but still didn't get any attention. I guess my only criticism is that this repository requires more active mainteners to avoid these situations.

@dvander
Copy link
Member

dvander commented Aug 16, 2021

Sorry that got missed. Feel free to ping me if you want a review on something. I can't guarantee I'll see or get to everything though, especially for things that I don't know anything about.

@rtxa
Copy link
Contributor

rtxa commented Aug 16, 2021

@dvander Okay, thanks!

FEDERICOMB96 added a commit to FEDERICOMB96/amxmodx that referenced this pull request Sep 16, 2021
Same as alliedmodders#981

Co-Authored-By: Franco Romaniello <romax.cs@gmail.com>
Arkshine pushed a commit that referenced this pull request Sep 17, 2021
Same as #981

Co-Authored-By: Franco Romaniello <romax.cs@gmail.com>

Co-authored-by: Franco Romaniello <romax.cs@gmail.com>
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.

None yet

5 participants