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

Modernize and improve scrollmsg.sma #579

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

OciXCrom
Copy link
Contributor

@OciXCrom OciXCrom commented Sep 13, 2018

The changes include:

  1. Added cvar amx_scrollmsg_color_red.
  2. Added cvar amx_scrollmsg_color_green.
  3. Added cvar amx_scrollmsg_color_blue.
  4. Added cvar amx_scrollmsg_only_dead (requested in About the scrollmsg and imessage plugins #571).
  5. Added cvar amx_scrollmsg_speed.
  6. Added cvar amx_scrollmsg_x_move_amount.
  7. Added cvar amx_scrollmsg_x_start_pos.
  8. Added cvar amx_scrollmsg_x_end_pos.
  9. Added cvar amx_scrollmsg_y_pos.
  10. Usage of create_cvar and bind_pcvar_* functions instead of the old ones.
  11. Change show_hudmessage to ShowSyncHudMsg to prevent overlapping messages if the repeat time is shorter.
  12. Consistent variable names, semicolons and brackets.
  13. Usage of AutoConfigExec - let me know if you want this to stay. I believe it's better this way rather than adding every cvar in amxx.cfg.

@ClaudiuHKS
Copy link
Contributor

ClaudiuHKS commented Sep 13, 2018

Very nice!
Just wanted to say that in my opinion, you should change if(, while(, switch( and for( to if (, while (, switch ( and for (. Also, change FLAG_A|FLAG_B|FLAG_C to FLAG_A | FLAG_B | FLAG_C.
This isn't something you must do, but, in my opinion, it makes the plugin more readable.

@OciXCrom
Copy link
Contributor Author

@ClaudiuHKS Thanks, I didn't notice them. I changed them.

@all85100
Copy link

Great, it's all done. How do I download the new plugins that have changed the content?

@all85100
Copy link

Usage of autoconfigexec, how did he change the plugin parameters to fixed? not using Amxx.cfg?

@all85100
Copy link

all85100 commented Sep 13, 2018

@OciXCrom Can you fix this bug?
#560
Have you tried ?
#572

@OciXCrom
Copy link
Contributor Author

@all85100 You can't download them just yet. This is a pull request, it's not yet added in AMXX. Be patient.

register_srvcmd("amx_scrollmsg", "setMessage", _, "<message> <frequency>");
bind_pcvar_string(get_cvar_pointer("hostname"), g_hostname, charsmax(g_hostname));

bind_pcvar_num(create_cvar( "amx_scrollmsg_color_red", "200", _, "Red color amount", true, 0.0, true, 255.0), g_amx_scrollmsg_color_red);
Copy link
Contributor

Choose a reason for hiding this comment

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

How about to implode it to one cvar amx_scrollmsg_color 200 100 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure really, both seem fine to me. Let's hear more opinions.

@all85100
Copy link

This modified plugin, I tested, did not show the contents of amx_scrollmsg "". Does it rely only on autoexecconfig generated files? The Amxx.cfg failed, Autoexecconfig generated files did not amx_
Scrollmsg parameters, My amxx.cfg file has amx_scrollmsg parameters and content, but it doesn't show.

It is best not to use AutoExecConfig, which is not very convenient for language translation and use.

@OciXCrom
Copy link
Contributor Author

@all85100 If a plugin uses AutoConfigExec() the cvars should be modified from the automatically generated .cfg file for that plugin. In this case amxx.cfg will have no effect because it's executed before the generated .cfg. It's not true that it can't be translated - it can. I don't see how amxx.cfg is better, especially when you need to add new cvars manually after each AMXX update.

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.

4 participants