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

When giving Levels directly (with /xp <level>L) player.get(Keys.TOTAL_EXPERIENCE) isn't updated #1322

Closed
2 of 3 tasks
BrainStone opened this issue Apr 25, 2017 · 7 comments
Closed
2 of 3 tasks
Assignees
Labels
status: accepted a confirmation that this is either an issue caused by us or something that should be added system: data

Comments

@BrainStone
Copy link
Contributor

When players recieve levels directly via the /xp <level>L [player] command the value of player.get(Keys.TOTAL_EXPERIENCE) remains unchanged. However the value is correct when reciving XP orbs or using the /xp <experience> [player] command.

I am currently running...

  • SpongeVanilla version: 1.11.2-6.0.0-BETA-253, 1.11.2-6.0.0-BETA-252 and 1.11.2-6.0.0-BETA-243
  • Java version: 1.8.0_121
  • Operating System:
  • Linux (Debian)
  • macOS
  • Windows

What is the expected result?
That the value of total experience is increased when giving levels directly.

What is the current result?
The value remains unchanged.

See also: SpongePowered/SpongeVanilla#312

@BrainStone
Copy link
Contributor Author

Also I noticed that when enchanting or repairing the value doesn't decrease.

@ryantheleach
Copy link
Contributor

Does receiving experience by an orb, after gaining it with the command, give you the correct total?

@BrainStone
Copy link
Contributor Author

@ryantheleach no it does not.

@stephan-gh
Copy link
Contributor

When I tried to reproduce @BrainStone's bug report on SpongeVanilla I noticed that the total experience and the experience levels are tracked separately (and also saved separately) in the internal Minecraft server. I'm not entirely sure in which cases it converts between these values (I haven't checked yet).

@BrainStone
Copy link
Contributor Author

Any updates so far?

@dualspiral
Copy link
Contributor

dualspiral commented May 8, 2017

I know what the issue is, I'm just unsure as to whether something should be done about it, and if so, whether it should be optional, because it's fundamentally a Vanilla issue - if indeed, it is an issue (and I'll explain my thinking in a moment).

The problem is, as stated, that the Minecraft server tracks total experience and levels (and the current fraction of experience between two levels) separately. When changing a player's total experience via the /xp command, Minecraft also ensures that the experience levels are updated. However, this does not happen the other way around, if the levels are updated, the xp is not. This doesn't just happen using the /xp command, you also observe this behaviour when you repair an item using an anvil and enchant an item using the enchantment table.

The issue is that the method on the EntityPlayer that adds to the total experience also does the calculations to work out the level change, but directly calling the level change does not change the experience, unless the level is set to zero. I see no other synchronisation points. It could be that this is intentional, because the XP can the become some sort of score, whilst the levels are expendable - and a lower level will still allow you to benefit from a lower level interval. However, given that a zero level returns you to zero experience, this seems unlikely.

The fix I have would be to ensure that, when levels are changed outside of the method that adds experience points, is to check to see if there is a mismatch between the current experience and the current level, and, if so, recalculate the total experience (using the current percentage to the next level as a guide). To be clear, I have this ready to go - but I don't know whether this patch should be in an optional mixin.

@parlough
Copy link
Contributor

parlough commented May 8, 2017

@dualspiral I don't see a point of it being optional. When would there be a need to control them separately. It just causes confusion for you as a plugin developer but also other plugins manipulating experience.

Having it as an optional mixin makes plugin developers not know what functionality to expect and is harder to develop for.

If Minecraft has these insurances we should too.

I think this would be a great addition, just not optionally :)

@stephan-gh stephan-gh added the status: accepted a confirmation that this is either an issue caused by us or something that should be added label May 17, 2017
JBYoshi added a commit to JBYoshi/Sponge that referenced this issue May 17, 2017
…levels.

This will also recalculate the XP whenever the player data is loaded. This should repair any broken calls from the past.

Fixes SpongePowered#1322.
JBYoshi added a commit to JBYoshi/Sponge that referenced this issue May 18, 2017
…levels.

This will also recalculate the XP whenever the player data is loaded. This should repair any broken calls from the past.

Fixes SpongePowered#1322.
JBYoshi added a commit to JBYoshi/Sponge that referenced this issue May 23, 2017
…levels.

This will also recalculate the XP whenever the player data is loaded. This should repair any broken calls from the past.

Fixes SpongePowered#1322.
JBYoshi added a commit to JBYoshi/Sponge that referenced this issue May 29, 2017
…levels.

This will also recalculate the XP whenever the player data is loaded. This should repair any broken calls from the past.

Fixes SpongePowered#1322.
@JBYoshi JBYoshi assigned JBYoshi and unassigned gabizou Jul 11, 2017
JBYoshi added a commit that referenced this issue Aug 5, 2017
…levels.

This will also recalculate the XP whenever the player data is loaded. This should repair any broken calls from the past.

Fixes #1322.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted a confirmation that this is either an issue caused by us or something that should be added system: data
Projects
None yet
Development

No branches or pull requests

7 participants