OpenModularTurretsTeam / OpenModularTurrets

Repo for OpenModularTurrets.
https://minecraft.curseforge.com/projects/openmodularturrets
Other
51 stars 35 forks source link

Turrets on Valkyrein Skies ship causes lag(I already know how to fix) #500

Closed Sunspot0411 closed 4 years ago

Sunspot0411 commented 4 years ago

Issue Description:

OMT turrets cause lag when placed on Valkyrien Skies ship

What happens:

when there are more than 5 or 6 turrets placed on a Valkyrien Skies ship it causes extreme lag.

What you expected to happen:

I have already find the cause, which is basically this code if (this.ticks % 5 == 0) { this.getWorld().notifyBlockUpdate(this.pos, this.getWorld().getBlockState(pos), this.getWorld().getBlockState(pos), 3); } this.ticks++;

and if (this.ticks % 5 == 0) { this.getWorld().notifyBlockUpdate(this.pos, this.getWorld().getBlockState(pos), this.getWorld().getBlockState(pos), 3); }

which are of line 158 and 180 in AbstractDirectedTurret class update() method. Simply reduce this update rate by 40 times would make the lag insignificant(at least for my computer it works). So I suggest just make (this.ticks % 5 == 0) configurable in config file, so that people can change the update rate slower depend on how good their computer is.

Steps to reproduce:

  1. Build a VS ship using Valkyrien Skies 1.0-alpha-1 mod
  2. put more than 6 turrets on that ship

Affected Versions (Do not use "latest"):

Your most recent log file where the issue was present:

This is not needed since the cause of this issue is already known.

Keridos commented 4 years ago

That sounds like an external bug, a simple notify shouldn't affect performance that so hard. I think this currently updates the rotation of the turret every 5 ticks. Gonna test what happens if I just comment that out.

Sunspot0411 commented 4 years ago

Such a problem exists not only on OMT but also on my PlasmaCannon mod. I have tested the performance, I made it update every 200 ticks and the lag becomes insignificant. This has something to do with how Valkyrien Skies ship is coded, which is something different from vanilla minecraft. My suggestion is simply to make this update rate configurable and set 5 as default. In this way you don't have to worry about how VS is coded and it would hurt nothing.

Keridos commented 4 years ago

It might be better to try something else. Iirc this method just updates the rotation of the turret which we should be able to do with a message and a client side update.

On 29.12.2019 13:56, Sunspot0411 wrote:

I have tested the performance, I made it update every 200 ticks and the lag becomes insignificant. This has something to do with how Valkyrien Skies ship is coded, which is something different from vanilla minecraft. My suggestion is simply to make this update rate configurable and set 5 as default. In this way you don't have to worry about how VS is coded and it would hurt nothing.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/OpenModularTurretsTeam/OpenModularTurrets/issues/500?email_source=notifications&email_token=AAU5UPNLYN7OY5AHGNZXJR3Q3CM6FA5CNFSM4J6E623KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHY6Z4A#issuecomment-569502960, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAU5UPLGEA3SOYGGCXOMQXLQ3CM6FANCNFSM4J6E623A.

Sunspot0411 commented 4 years ago

OK, but if you wanna try something else you will need to talk with Triod aka thebest108 to come up with some ideas. Cause you need to know how VS ships do update().

Keridos commented 4 years ago

Fixed via https://github.com/OpenModularTurretsTeam/OpenModularTurrets/commit/f4cdd7cdc7c23e5d46854cbe7046eff8dfe9fe90