Fixed runas ~op for more plugins compatibility.

[t3hk0d3]

Now runas ~op operated using setOp(true) instead of Proxy object

[zml2008]

You should probably try finally the command execution and restoration of op level incase the command throws an uncaught
exception

[LadyCailin]

I'm concerned with this method, because this is not reliable in the case that the server crashes in between the time period that you op the player and you de-op them. One case I can think of is that the command run causes an infinite loop, so the server owner hard stops the java process. In this case, the player remains op after the reset. Is there a particular problem that this method solves? Essentially you're doing the same thing that I am, just in a less sledgehammer way.

[t3hk0d3]

@zml2008, thank you. you are right. I totally forgot about that :)

@wraithguard01 Thank you, i got your point. There is my new idea:
OPs stored in ServerConfigurationManager's h (Set) property.

Minecraft player OPing look like this:

this.h.add(playerName.toLowerCase()); // add to OPs set
this.n(); // store h to ops.txt

We just should get h field using reflection and put/remove player name into set.

Since CommandHelper have minecraft-server and craftbukkit in dependencies this is no problem :)

[LadyCailin]

Hmm, well, as I'm sure you well know, I'm not against using reflection and evilness when needed, but I try to reserve its use only when that is strictly the only time it's needed. Secondly, the CraftBukkit dependency is reserved for experimental features, or features that just don't have a hook in Bukkit. I still don't really get what this solves that isn't already solved with the proxy. Though the argument may be that the proxy class is a bit unstable (if they change the name of the isOp function, I can't catch that at compile time), your method of getting an obfuscated and undocumented function is much more unstable. What exactly are you trying to fix?

[t3hk0d3]

Okay let me explain:

CommandHelper executes some command, "pex user t3hk0d3 group set Admins" for example as ~op

Problem what some plugins (and PEX too) have some internal abstraction from Bukkit and don't pass Player object internally.

ProxyPlayer -> some command handler -> playerName = ProxyPlayer.getName() -> .... -> is op check - Bukkit.getServer().getPlayer(playerName).isOp()

Result would be isOp == false

Right, but both methods (ProxyPlayer and hacking minecraft server internals) actually bogus. setOp would be fine, until this method do I/O and persist into ops.txt.

I cleary understand your worries about creating unmaintainable code, but current solution (Proxy object) don't work in all cases and not compatible with some plugins.

On other hand, like @zml2008 mentioned before, try-finally may be enough to catch most command execution failures.
Since Minecraft server actually mostly single threaded there is no much cases when complete fatal error may cause in another thread.

[LadyCailin]

Ahh, I see! This explains a lot actually. Ok, well, as long as we don't write out the op setting to disk, I'm ok with this. I'll pull this in later tonight when I get home.

[zml2008]

Glowstone is not all that single-threaded. The playerlistfiles will
eventually get their own threading for file saving, and at least console
command execution occurs in another thread.

[t3hk0d3]

Im thinking about making Player.setOp as fallback option. Ie if Bukkit.getServer() aint instance of CraftServer than use Player.setOp

[LadyCailin]

Actually I was going to do that, and make the mock player anyways.

On Sep 20, 2011, at 8:59, t3hk0d3reply@reply.github.com wrote:

Im thinking about making Player.setOp as fallback option. Ie if Bukkit.getServer() aint instance of CraftServer than use Player.setOp

Reply to this email directly or view it on GitHub:
#3 (comment)

[t3hk0d3]

I'm included everything we discussed in IRC (i hope i didn't forget anything). Also im incuded @albatrossen improvements with little refactoring :)