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

Solution to #103 #105

Closed
wants to merge 2 commits into from
Closed

Solution to #103 #105

wants to merge 2 commits into from

Conversation

iViscosity
Copy link

@iViscosity iViscosity commented Aug 16, 2017

No description provided.

@Nayruden
Copy link
Member

Thank you for your interest in making ULX/ULib better for all users! We appreciate all the contributions our users send in. However, I'm denying this PR for two reasons.

  1. There are other commands that have the same issue, such as adduserid and userdenyid. While your approach would work, it is not a clean solution to apply to all affected commands. Which brings us to...

  2. The ideal solution in my mind is to implement a new argument type in commands.lua. This new type would be very similar to PlayerArg, except that it would target offline users via steamid. Though it's been on my to-do list a long time, I haven't implemented this yet simply because it seems to me that if an admin is abusing banid, adduserid, etc, then that admin should have his/her powers removed.

@Nayruden Nayruden closed this Aug 16, 2017
@Codingale
Copy link

I believe there's also a bit of an issue where you can banid someone who was banned longer than you're allowed and either lessen the ban or completely unban them.

@Nayruden
Copy link
Member

I can see the point you make @Codingale, but this is why banid defaults to superadmins, while ban defaults to admins.

@Codingale
Copy link

Just noted, this actually wouldn't fix it anyways. It throws an error:

[ERROR] addons/ulib-v2_63/lua/ulib/shared/player.lua:165: attempt to index local 'target' (a nil value)
           1. getUsers - addons/ulib-v2_63/lua/ulib/shared/player.lua:165
            2. call - addons/ulx-v3_73/lua/ulx/modules/sh/util.lua:130
             3. __fn - addons/ulib-v2_63/lua/ulib/shared/commands.lua:943
              4. execute - addons/ulib-v2_63/lua/ulib/shared/commands.lua:1323
               5. unknown - addons/ulib-v2_63/lua/ulib/shared/commands.lua:1351
                6. Run - lua/includes/modules/concommand.lua:54
                 7. unknown - addons/ulib-v2_63/lua/ulib/shared/commands.lua:1365
                  8. unknown - lua/includes/modules/concommand.lua:54

@iViscosity
Copy link
Author

That was a fault on my end. I fixed it for myself but I didn't edit this because it was already closed.

@Codingale
Copy link

is it on your repo? @iViscosity I'd like to add it but am too lazy to find out how to check if a group can target another..

@iViscosity
Copy link
Author

iViscosity commented Aug 29, 2017

It is not but I can add it real fast

EDIT: Actually, the one I have on there should work.

@Codingale
Copy link

Codingale commented Aug 29, 2017

I think I tried that one too, also I think running ulx banid via server console throws an error.

Edit:


[ERROR] addons/ulx-v3_73/lua/ulx/modules/sh/util.lua:130: attempt to call method 'GetUserGroup' (a nil value)
  1. call - addons/ulx-v3_73/lua/ulx/modules/sh/util.lua:130
   2. __fn - addons/ulib-v2_63/lua/ulib/shared/commands.lua:943
    3. execute - addons/ulib-v2_63/lua/ulib/shared/commands.lua:1323
     4. unknown - addons/ulib-v2_63/lua/ulib/shared/commands.lua:1351
      5. unknown - lua/includes/modules/concommand.lua:54

Also doesn't work (Unless a full restart is required)

@iViscosity
Copy link
Author

Yeah running through console will cause an error now that I think about it. Let me fix that. (Also I don't know if we should be having convo through here :P)

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

3 participants