BuildCraft|Core

BuildCraft|Core

7M Downloads

Fake player performance issue

TheAndrey opened this issue · 39 comments

commented

I noticed lags on the server (5-6 TPS). Warmroast shows that a problem in TileQuarry. Why you use FakePlayer for destruction of blocks? Why it can't be done as it was in the previous versions - directly through setBlockToAir()?

Screenshot

commented

ppl can grief if you dont use FakePlayer, WorldGuard

commented

@008 I added to a code a mode special Bukkit event. They are processed by only one plug-in. It considerably reduces loading and does to impossible grief regions where [buildcraft] is added to members. The plug-in looks who the owner of quarry and decides to give access or not. Here so it becomes :)

commented

The quarry is not the one lagging there, it's whatever happens inside CraftEventFactory.callBlockBreakEvent that takes all of the time.

commented

@hea3ven The quarry uses FakePlayer which throws an event of destruction of the block. Why so to do if it is possible to break the block without use of the FakePlayer?
FakePlayer is necessary only where it is impossible to do without it. For example, in autoclickers which use items because the onItemUse() method demands object instance of EntityPlayer.

commented

How is my example bad? If you don't care about block updates caused by a fake player (machines etc), just skip them...

commented

like @008 said, it is needed to throw the BreakEvent, so that other mods can react acordingly to the blocks being broken.

commented

Only plug-ins think that the block is broken by the real player. And the fake breaks blocks much more often (if many quarries), than real players. Big load of the one-thread server turns out.

commented

This is recommended behaviour, expand this list further and think about reporting it to bukkit or plugin that causes the overhead.
As you can see it is not overhead of Buildcraft but Bukkit's handler so I think that one of plugins is there an issue.

commented

All my plug-ins are well optimized. In timings everything with plug-ins well. A problem that the event is fires too often. It in fact as additional players. The server is calculated on 100 slots, and 200+ players online turn out.

commented
if (player instanceof FakePlayer) return;
commented

@Lunatrius Very bad example.

commented

How many times is the BlockBreakEvent being done? VisualVM shows that, not
sure if WarmRoast does.

2015-11-03 18:04 GMT+01:00 Andrey [email protected]:

@Lunatrius https://github.com/Lunatrius Very bad example.


Reply to this email directly or view it on GitHub
#3109 (comment)
.

commented

@asiekierka
VisualVM

Essentials gc (75 players online):
Essentials gc

commented

You should be able to turn on column samples by right-clicking the bar and checking samples.

commented

@Kubuxu Thanks. Screenshot updated.

commented

I tell not about compatibility, and about bad use a fake of players.
My messages about Bukkit and grief are addressed to those who considers that without fake of the player there will be no protection against grief.

It is possible to use Forge event system that other modes could trace change of blocks the mechanism.

commented

It's the absolutely correct use. I'm saying that I'm breaking a block, so I
emit a break block event.

2015-11-04 15:42 GMT+01:00 Andrey [email protected]:

I tell not about compatibility, and about bad use a fake of players.


Reply to this email directly or view it on GitHub
#3109 (comment)
.

commented

Why you emit a break block event? Unless without it badly works?

commented

I doubt I can actually fix this, it's a plugin performance issue. What I can do is try and make break events created a bit less often.

commented

@asiekierka why it is impossible to break blocks directly by means of setBlock() and getDrops()? In old BuildCraft versions was quite so. The majority of modders so do and there are no problems.
The only thing the case where is necessary the fake player - an active transport pipe. There they it is used that the installed block turned facing the player. In all other cases we can place blocks by means of setBlock().

Try to minimize use a fake of players. Use only where well not to do without them in any way (difficult actions the demanding participations of the player).

commented

maybe a midway solution is possible? add a config option to dissable the use of the fake player but leave it on by default as it can (and will) be used to bypass permission plugins/mods

commented

@AEnterprise It would be good...

commented

@TheAndrey you server would bee all griefed and Quarry will be griefed by Quarry

commented

@008 You read my message? I solved this problem for a long time. A fake players for me - the last century.

commented

How do you identify the owner of a Quarry?

commented

I write down in NBT TileEntity UUID of the player who installed this block. Quarry (or other mechanism) fires custom Bukkit event which contains information about of owner of the mechanism and the block which the mechanism wants to change. My special plug-in processes this event and by means of API WorldGuard (or the friend of a plug-in of the protected zones) whether checks there is in the region an owner of the mechanism. If is - the mechanism is allowed to break the block.

Also there is the second option of check - on locations of the block of the mechanism. If the block of the mechanism and the block which wants to change the mechanism are in the same region of WorldGuard - it is allowed to change the block. Grifer will definitely not be able to place the mechanism in others region.

So safely and at the same time conveniently for players - I put mechanism and it works (nobody needs to be added to the region). :)

Such system of check of access was thought up by @Shevchik I supported this idea.

commented

The second option will not work. Players can place the Quarry right outside the WorldGuard region and the Quarry could still eat large parts of a base. It is important that you keep the owner information.

commented

@SandGrainOne A year it is option works. Quarry won't be able to change units in the protected region as its block (that which placed by the player and connects to it engines) is outside the region. The rule "in the same region" isn't executed here, means the mechanism won't get access to change of blocks.
Image

commented

@TheAndrey FakePlayers are the Forge-sanctioned way of doing block breaking. Compatibility with Forge is our primary concern, Bukkit is secondary. We could add a config option, I suppose.

And no, we cannot make this faster. We are already removing every block that is found to not be breakable from our lookup lists.

commented

without it other mods can't prevent the quarry for from breaking blocks, for example protected blocks like a TE strongbox with an lock on it. only it's owner is allowed to break the block and not the quarry

commented

@AEnterprise Such blocks usually make unbreakable, so, any mechanism won't be able to break them.

commented
commented

@TheAndrey nope that's not how most of those work as the owner can still break them, some mods also listen to the event to handle drops or do special things

commented

@AEnterprise It is complete of other mods in which there are mechanisms the changing blocks. They will successfully break such block because do it directly without use the player's fake. Therefore in such method of protection the sense isn't present.

@asiekierka I ask to add possibility of switch-off of sending events. Events aren't necessary to me because of loading. It will be so good for all.

commented

@TheAndrey That's not how it should be done. -_-
Fake players are the correct way of going about doing this and firing the event is also a necessity if your mod is to be respected by other mods.
Whatever is causing your problem is not to do with BuildCraft, but rather something that cauldron/whatever is doing. I suggest you try without bukkit plugins and try without cauldron (use forge).

commented

@simon816 If to follow your logic, the fake players needs to be used where it is only possible. For example, TNT changes blocks. Why not to add the fake player to it?

The machine, is the machine. And players it is the player. Why the machine pretends, what it is the player if actually that isn't?

commented

There is custom event for TNT explosion. There isn't one for BC because BC just breaks blocks and BlockBreakEvent as it is called is used to ensure that BC does that only in area that it should. Your performance problems aren't caused because BC does something wrong but because one of your handlers have too large overhead.

commented

Say players 'explode', as in there is a key on the keyboard which will explode the player and the surrounding area. Then having a fake player for TNT could make sense, since that's a player action that could be simulated using code.
BuildCraft uses a fake player because what's the quarry doing? breaking blocks. Players break blocks, breaking blocks in an action a player chooses to do. A quarry simulates the action otherwise done by players, hence the use of a fake player to represent the abilities of that player on the blocks it digs. Having a fake player gives Minecraft along with other mods the power to control what the quarry can do.
BuildCraft won't be removing it's fake player (that's been around for years), no matter how convincing of an argument you make, right @asiekierka?

commented

This projects issue list is not the right place to discuss whether forge and forge mods are doing these things in the best possible way or not. Take this up with the team that maintains Forge.