Addition and Modification in OnPlayerCommandText.pwn by Specifer · Pull Request #842 · LVPlayground/playground

Specifer · 2021-01-23T17:25:35Z

Change in /admins and addition of /vips.

Tested.

Change in /admins
I have added a level Temp. Admin in /admins for players to differentiate full-time admins and part-time, Since temp admins operate at limited power. I felt this knowledge should be transparent to fellow players. Tested it thoroughly. And admins can see who temp'ed the person while players only see temp-admin. Also, added Developers to this list since it was long overdue? What do you think?

Addition of /vips

Just similar to /admins but shows also higher level (i.e) VIP, Developer, Administrator, and Manager.

Change in /admins and addition of /vips. Tested..

FreakinsPL · 2021-01-24T02:45:09Z

My opinions

Make a separate cmd to see ingame developers (/devs)rather than including them in /admins coz they don't have admin rights.
I don't think so there's a reason of making /vips, why ppl wants to see who is vip and who is not? :)

@OttoRocket-LVP @Specifer

OttoRocket-LVP · 2021-01-24T12:37:38Z

+                format(playerLevel, sizeof(playerLevel), "Developer");      
            if (Player(player)->isAdministrator() && !Player(player)->isManagement())
-                format(playerLevel, sizeof(playerLevel), "Administrator");
+                format(playerLevel, sizeof(playerLevel), "Administrator");  


NIT: You added a enter here

OttoRocket-LVP · 2021-01-24T12:38:10Z

            // If the user was temp'd, show admins who temp'd the player.
-            if (tempLevel[player] == 1 || tempLevel[player] == 2) {
-                if (Player(playerid)->isAdministrator())
+            {     


What happened here? There's brackets without an if statement or whatso ever.

You might've made a mistake?

OttoRocket-LVP · 2021-01-24T12:39:13Z

-            if (tempLevel[player] == 1 || tempLevel[player] == 2) {
-                if (Player(playerid)->isAdministrator())
+            {     
+                if (Player(playerid)->isAdministrator() && Player(playerid)->isManagement()) 


This isAdministrator is redundant here. If you're management you're always an admin

OttoRocket-LVP · 2021-01-24T12:39:43Z

+                    }
+                else {
+                    if (tempLevel[player] == 2) 
+                    format(message, sizeof(message), " %s (Id:%d) - {FF8E02}Temp.Admin",


Do we really want to display that they're temporary?

If so we'd want to spell it out fully.

OttoRocket-LVP · 2021-01-24T12:40:31Z

+
+
+            if (Player(player)->isDeveloper())
+                format(playerLevel, sizeof(playerLevel), "Developer");      


Do we really want to have Developers among this list?

It'll probably be unclear to players that they do not have administrator rights and thus can't help them with houses reports and such

Ah, but they can report bugs to devs directly?

If you want, I can remove it.

A developer has no administrative capability so I think adding them to /admins would be missleading.

OttoRocket-LVP · 2021-01-24T12:42:38Z

Thank you for your modification as always!

Are you sure you fully tested this @Specifer? (With all levels mentioned?) I think there might be some minor issues.

Also please see inline comments.

Specifer · 2021-01-24T12:44:25Z

Yes Tested fully by myself.
I'll also recheck once it is in staging with Freakins.

Specifer · 2021-01-25T10:52:46Z

Everything good?

OttoRocket-LVP · 2021-01-25T18:38:13Z

            if (!Player(player)->isAdministrator()) continue;
            if (IsPlayerAdmin(player)) continue;

+


NIT: Some redundantent enters here

OttoRocket-LVP · 2021-01-25T18:39:18Z

+
+
+            if (Player(player)->isDeveloper())
+                format(playerLevel, sizeof(playerLevel), "Developer");      


A developer has no administrative capability so I think adding them to /admins would be missleading.

OttoRocket-LVP · 2021-01-25T18:44:51Z

                player, playerLevel);

            // If the user was temp'd, show admins who temp'd the player.
-            if (tempLevel[player] == 1 || tempLevel[player] == 2) {


What is your reason to swap these ifs arounds?

Isn't it right?

OttoRocket-LVP · 2021-01-25T18:45:44Z

+                        Player(player)->nicknameString(), UserTemped[player], player, playerLevel); 
+                    }
+                else {
+                    if (tempLevel[player] == 2) 


This condition is caught in the if statement above. This code should never be reached as I'm reading it?

OttoRocket-LVP · 2021-01-25T18:46:28Z

        return 1;
    }

+    if (strcmp(cmd, "/vips", true) == 0) {


Wouldn't it be nice to have this in JavaScript instead?

We'd like to move all code to JavaScript

Um maybe for the moment? can we keep pwn. I'll make js for both /admins and /vip altogether.

OttoRocket-LVP · 2021-01-25T18:47:29Z

+            if (Player(player)->isVip())
+                format(playerLevel, sizeof(playerLevel), "VIP");
+            if (Player(player)->isDeveloper())
+                format(playerLevel, sizeof(playerLevel), "VIP / Developer");          


A Developer doesn't always have to be a VIP. I believe showing "Developer" would be enough

OttoRocket-LVP · 2021-01-25T18:49:13Z

+        SendClientMessage(playerid, COLOR_ORANGE, "List of VIPs online in Las Venturas Playground");
+
+        new message[128], vipCount = 0, playerLevel[30];
+        for (new player = 0; player <= PlayerManager->highestPlayerId(); player++) {


I believe calling the variable "playerId" would be more suitable.

"player" would assume that it's the entity player.

Specifer · 2021-02-01T03:07:42Z

Dude holsje? Reply if this new commit has resolved the errors.

FreakinsPL · 2021-02-23T13:24:42Z

It would be good if we make this /vips cmd simple as possible.

Even if the players are administrators, managers or a developers. The /vips command should show only -VIP after the player name

example:-

Vip connected:
Freakins -VIP
Specifer -VIP
Holsje -VIP
Xilent -VIP

Specifer added 2 commits January 23, 2021 22:48

Update OnPlayerCommandText.pwn

c860aeb

Change in /admins and addition of /vips. Tested..

Update OnPlayerCommandText.pwn

5234dba

OttoRocket-LVP reviewed Jan 24, 2021

View reviewed changes

Update OnPlayerCommandText.pwn

1bf56cc

OttoRocket-LVP reviewed Jan 25, 2021

View reviewed changes

Update OnPlayerCommandText.pwn

3b5aae1



		if (Player(player)->isDeveloper())
		format(playerLevel, sizeof(playerLevel), "Developer");

		if (!Player(player)->isAdministrator()) continue;
		if (IsPlayerAdmin(player)) continue;

Conversation

Specifer commented Jan 23, 2021

Uh oh!

FreakinsPL commented Jan 24, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OttoRocket-LVP commented Jan 24, 2021

Uh oh!

Specifer commented Jan 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Specifer commented Jan 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Specifer commented Feb 1, 2021

Uh oh!

FreakinsPL commented Feb 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Specifer commented Jan 24, 2021 •

edited

Loading