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

API8 fixes #2609

Merged
merged 47 commits into from May 18, 2020
Merged

API8 fixes #2609

merged 47 commits into from May 18, 2020

Conversation

Faithcaio
Copy link
Contributor

a bunch of fixes for the API8 implementation

@@ -49,8 +49,8 @@ public SpongeEntityDamageSourceBuilder entity(final Entity entity) {
public EntityDamageSource build() throws IllegalStateException {
checkState(this.damageType != null, "Damage type cannot be null!");
checkState(this.reference.get() != null);
final net.minecraft.util.EntityDamageSource damageSource =
new net.minecraft.util.EntityDamageSource(this.damageType.getId(), (net.minecraft.entity.Entity) this.reference.get());
final net.minecraft.util.EntityDamageSource damageSource = // TODO damageType String is used for the translation key!
Copy link
Member

Choose a reason for hiding this comment

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

this is negotiated based on the mixin and damage source to type provider.

Copy link
Member

@gabizou gabizou left a comment

Choose a reason for hiding this comment

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

Turn on local variable can be final inspection and fix them. There's also some areas where it just looks really long or nested method calls that should be local variables (the compiler can take care of inlining).

if (fromClientDimId == toClientDimId) {
final int fakeDimId;
switch (fromClientDimId) {
if (fromType == toType) {
Copy link
Member

Choose a reason for hiding this comment

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

@Zidane is this right with dimension handling? I'd imagine the type could be the same but the dimension id assigned by the WorldManager could be different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to send a fake respawn packet to vanilla clients in case the dimension type is not changing.
This entire teleport things needs extensive testing anyways once we can actually start a server.
Maybe its not needed anymore?

Copy link
Member

Choose a reason for hiding this comment

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

No you still have to fake sent a dimension change else you must send an unload packet for every chunk.

That is not including the other fun things the client sets based on the dimension.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is still mentioned in the protocol wiki

@gabizou unsure about the ChunkManagerMixin_Tracker
where the target is a lambda
@Faithcaio Faithcaio added the status: needs review a code review is needed label May 16, 2020
@gabizou gabizou merged commit 9466952 into api-8 May 18, 2020
@Faithcaio Faithcaio deleted the api8/fixes branch May 19, 2020 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs review a code review is needed version: 1.16 (u) API: 8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants