Skip to content
This repository was archived by the owner on May 29, 2022. It is now read-only.

Few little optimisations#365

Open
crafter23456 wants to merge 25 commits into
CobbleSword:masterfrom
crafter23456:test
Open

Few little optimisations#365
crafter23456 wants to merge 25 commits into
CobbleSword:masterfrom
crafter23456:test

Conversation

@crafter23456
Copy link
Copy Markdown
Contributor

@crafter23456 crafter23456 commented Feb 28, 2022

Description

Use Vanilla Minecart Speeds

  • Minecart is driving like it should, I didnt made any tests how far it fly etc only if it works normal

Optimize World Time Updates PaperMC/Paper@70e091b

  • time went normal, /time set X works. so should be fine

I bumped some dependencies and updated the velocity repo cause the link was broken. Log4j should be fixed with only the core in fact Paper did only the trove4j and log4j core in their 1.8.8 log4j bugfix.
PaperMC/Paper@2894af0#diff-9f5fad4c579c54b5c3b0080702b87b856679a932ee4338409dab8b8cebe06292R16
Besides of that I got inspired by the spigot 1.18.1 pom and changed gitdescribe to scriptus.
All builds fine. No console errors.

Checklist:

  • I have reviewed my code thoroughly.
  • I have tested my code.
  • My changes generate no new warnings

@HowardZHY
Copy link
Copy Markdown

will @crafter23456
Use Vanilla Minecart Speeds
have an influence with Train Carts pl ?

@crafter23456
Copy link
Copy Markdown
Contributor Author

will @crafter23456 Use Vanilla Minecart Speeds have an influence with Train Carts pl ?

dunno. it only changes the speed back to default when the cart is flying...

Comment thread NachoSpigot-Server/pom.xml Outdated
Comment thread pom.xml Outdated
Comment thread pom.xml Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/TileEntitySign.java Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/TileEntitySign.java Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/ChunkProviderServer.java Outdated
crafter23456 and others added 3 commits March 1, 2022 00:31
…tServer.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
…viderServer.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
…tySign.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
Copy link
Copy Markdown
Member

@HeathLoganCampbell HeathLoganCampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal I think this is a lot of changes in one pull request which makes things really messy, I would suggest breaking them up especially since dep bumping can be high risk

Comment thread NachoSpigot-API/pom.xml
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-iostreams</artifactId>
<version>2.17.1</version>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to remove these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say. Paper only use the log4j core and the other thing I don't remember currently but we have that too. And my anticheat has a fix for log4j which only activates if the server didn't done it before. Isn't a 100% trust but if paper do it, I guess

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it would be safe, I don't think it's recommended to combine multiple versions of a library.
IMO, this should be reverted. There's no reason for this.

Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/ChunkProviderServer.java Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/ChunkProviderServer.java Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/ItemBoat.java Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/PlayerConnection.java Outdated
Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/TileEntitySign.java Outdated
crafter23456 and others added 7 commits March 1, 2022 00:38
Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
…CraftBlockState.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
…CraftBlockState.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
…CraftBlockState.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
…CraftBlockState.java

Co-authored-by: Elier <71361901+Elierrr@users.noreply.github.com>
@crafter23456
Copy link
Copy Markdown
Contributor Author

okay thats quiet weird. on my fork it builds, and here not. in fact that it builded here until we commited entityhuman out...

@ghost
Copy link
Copy Markdown

ghost commented Mar 2, 2022

okay thats quiet weird. on my fork it builds, and here not. in fact that it builded here until we commited entityhuman out...

That’s because it can’t find bungeecord-chat and you have it cached

@Sculas
Copy link
Copy Markdown
Collaborator

Sculas commented Mar 2, 2022

Did you add that dependency? If not, we might have a problem again. Need to look into a Maven repository sometime for this.

@crafter23456
Copy link
Copy Markdown
Contributor Author

okay thats quiet weird. on my fork it builds, and here not. in fact that it builded here until we commited entityhuman out...

That’s because it can’t find bungeecord-chat and you have it cached

no i mean on my repo on GH with the same GH actions xd on my server yes thats cached

Did you add that dependency? If not, we might have a problem again. Need to look into a Maven repository sometime for this.

I changed the velocity repo link cause it was outdated and the normal build script but it builded here already until we added a commit which isnt related if the api doesnt get found

@ghost
Copy link
Copy Markdown

ghost commented Mar 2, 2022

Did you add that dependency? If not, we might have a problem again. Need to look into a Maven repository sometime for this.

It’s caused by the removal of the sonatype parent, I have a fix in crafter23456#3

@crafter23456
Copy link
Copy Markdown
Contributor Author

there still open conversations from heath

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-iostreams</artifactId>
<version>2.17.1</version>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it would be safe, I don't think it's recommended to combine multiple versions of a library.
IMO, this should be reverted. There's no reason for this.

Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/ItemBoat.java Outdated
@sadcenter
Copy link
Copy Markdown
Contributor

@Lucaskyy I personally like the time per player feature, and it has no performance impact so yea. I used time per player and it worked on nacho

@crafter23456
Copy link
Copy Markdown
Contributor Author

Personal I think this is a lot of changes in one pull request which makes things really messy, I would suggest breaking them up especially since dep bumping can be high risk

i split the boat thing into another PR. so now its not that much

Comment thread NachoSpigot-Server/src/main/java/net/minecraft/server/TileEntitySign.java Outdated
@HowardZHY
Copy link
Copy Markdown

HowardZHY commented Mar 26, 2022 via email

@crafter23456
Copy link
Copy Markdown
Contributor Author

@HeathLoganCampbell I removed the new world time patch. Is there anything else I missed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants