Team-EnderIO/EnderIO
1
0
Fork 0
Code Issues 114 Pull requests 13 Projects Releases 79 Packages Wiki Activity Actions

Rework Experience Calculation logic #547

Merged
dphaldes merged 4 commits from dev/rewrite-xp-logic into dev/1.20.1 2023-11-23 21:53:53 +00:00
Conversation 15 Commits 4 Files changed 5 +90 -85
dphaldes commented 2023-10-23 10:14:07 +00:00 (Migrated from github.com)
Copy link

Description

Changes the xp calculations to remove recursive calls and use much straightforward formulae

(Closes issues indirectly by removing all stack overflow recursions)
Closes #543
Closes #313

  • My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you).
  • I have performed a self-review of my own code.
  • I have commented my code in areas it may be challenging to understand.
  • I have made corresponding changes to the documentation.
  • My changes are ready for review from a contributor.
# Description Changes the xp calculations to remove recursive calls and use much straightforward formulae (Closes issues indirectly by removing all stack overflow recursions) Closes #543 Closes #313 - [X] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you). - [X] I have performed a self-review of my own code. - [X] I have commented my code in areas it may be challenging to understand. <!-- (Although we prefer code that is readable instead of over-commented) --> - [X] I have made corresponding changes to the documentation. - [X] My changes are ready for review from a contributor. <!-- Thanks to: https://embeddedartistry.com/blog/2017/08/04/a-github-pull-request-template-for-your-projects/ for the building blocks of this template -->
dphaldes commented 2023-10-23 10:15:24 +00:00 (Migrated from github.com)
Copy link

I have also tried to clean function names, but it still requires some work. Let me know if you would like me to change docs/names

I have also tried to clean function names, but it still requires some work. Let me know if you would like me to change docs/names
dphaldes commented 2023-10-23 14:05:44 +00:00 (Migrated from github.com)
Copy link

#363 will require an update if this PR is merged first (else the other way around)

#363 will require an update if this PR is merged first (else the other way around)
maxinglo commented 2023-10-30 06:55:59 +00:00 (Migrated from github.com)
Copy link

If the fluid xp use int type can only contain about 4000 levels. maybe it's a problem?

If the fluid xp use int type can only contain about 4000 levels. maybe it's a problem?
Rover656 commented 2023-11-11 13:42:29 +00:00 (Migrated from github.com)
Copy link

Maybe it is smart to switch to long in some places, similarly to #329

Maybe it is smart to switch to long in some places, similarly to #329
dphaldes commented 2023-11-11 17:02:40 +00:00 (Migrated from github.com)
Copy link

Overflows should not happen anymore, as we don't use recursions anymore. Switching to long may not fix anything as vanilla max level is an INT

Overflows should not happen anymore, as we don't use recursions anymore. Switching to long may not fix anything as vanilla max level is an INT
Rover656 commented 2023-11-12 20:38:48 +00:00 (Migrated from github.com)
Copy link

Stack overflows are avoided, yes however if we overflow an integer then we might find ourselves dealing with weird negatives

Stack overflows are avoided, yes however if we overflow an integer then we might find ourselves dealing with weird negatives
dphaldes commented 2023-11-13 14:01:43 +00:00 (Migrated from github.com)
Copy link

I will change the places where we store raw experience points into longs. The levels need to stay as an integer so that it is compatible with vanilla.

I will change the places where we store raw experience points into longs. The levels need to stay as an integer so that it is compatible with vanilla.
👍 1
GotoFinal (Migrated from github.com) reviewed 2023-11-15 22:25:38 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
GotoFinal (Migrated from github.com) commented 2023-11-15 22:25:38 +00:00
Copy link

any chance to make this long? and accept long in getTotalLevelFromXp, as usually xp is just used as levels + the xp, so this method breaks faster than actual levels you can get, and I still think that for heavy modded moddpack we really need to start going away from ints, so many issues in so many mods as people just keep going higher.
This breaks at 21863+ level, that is very high, but i feel like its doable with how many weird mods this can be combined with

any chance to make this long? and accept long in `getTotalLevelFromXp`, as usually xp is just used as levels + the xp, so this method breaks faster than actual levels you can get, and I still think that for heavy modded moddpack we really need to start going away from ints, so many issues in so many mods as people just keep going higher. This breaks at 21863+ level, that is very high, but i feel like its doable with how many weird mods this can be combined with
dphaldes (Migrated from github.com) reviewed 2023-11-16 05:50:21 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
dphaldes (Migrated from github.com) commented 2023-11-16 05:50:21 +00:00
Copy link

Yeah that is the plan. The only issue Player#giveExperiencePoints takes int input. I will replace rest of the values with longs

Yeah that is the plan. The only issue `Player#giveExperiencePoints` takes int input. I will replace rest of the values with longs
dphaldes (Migrated from github.com) reviewed 2023-11-16 05:54:42 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
dphaldes (Migrated from github.com) commented 2023-11-16 05:54:42 +00:00
Copy link

Another issue that fluidStacks used to store XP use integers as well.

Another issue that fluidStacks used to store XP use integers as well.
GotoFinal (Migrated from github.com) reviewed 2023-11-16 10:01:25 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
GotoFinal (Migrated from github.com) commented 2023-11-16 10:01:25 +00:00
Copy link

Ah, didn't notice existing comment, sorry, using long for this would allow to easier future proof to hopefully improve containers later too, but for now would at least give easy way to check if XP will overflow and limit operations to only fill containers up to int max and not worry about it overflowing when calculating

Ah, didn't notice existing comment, sorry, using long for this would allow to easier future proof to hopefully improve containers later too, but for now would at least give easy way to check if XP will overflow and limit operations to only fill containers up to int max and not worry about it overflowing when calculating
dphaldes (Migrated from github.com) reviewed 2023-11-16 10:03:07 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
dphaldes (Migrated from github.com) commented 2023-11-16 10:03:07 +00:00
Copy link

Any suggestions on how I should do that ?

Any suggestions on how I should do that ?
GotoFinal (Migrated from github.com) reviewed 2023-11-16 17:20:21 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
GotoFinal (Migrated from github.com) commented 2023-11-16 17:20:21 +00:00
Copy link

I think i had that in my older PR https://github.com/Team-EnderIO/EnderIO/pull/329/files#diff-f063d8fbab0bf1ca6d6fe016dbb81cd1fd3609a4bcc9d7d4ca8112f8b966dee1R103 but for sure need to check more places in code where xp is used.

I think i had that in my older PR https://github.com/Team-EnderIO/EnderIO/pull/329/files#diff-f063d8fbab0bf1ca6d6fe016dbb81cd1fd3609a4bcc9d7d4ca8112f8b966dee1R103 but for sure need to check more places in code where xp is used.
dphaldes (Migrated from github.com) reviewed 2023-11-16 17:22:00 +00:00
src/main/java/com/enderio/base/common/util/ExperienceUtil.java
@ -1,6 +1,5 @@
package com.enderio.base.common.util;
dphaldes (Migrated from github.com) commented 2023-11-16 17:22:00 +00:00
Copy link

Looks good! I will convert them over :)

Looks good! I will convert them over :)
Rover656 (Migrated from github.com) approved these changes 2023-11-23 21:51:29 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

This looks great, thanks a lot to both chonky and GotoFinal for working on this issue!

This looks great, thanks a lot to both chonky and GotoFinal for working on this issue!
Sign in to join this conversation.
Reviewers
No reviewers
Rover656
Labels
Clear labels   
Area-Assets

   
Area-Backend

Backend work not usually visible to players.

  
Area-Conduits

Everything involving conduits

  
Area-Datapacks

   
Area-Lang

   
Area-Mod Compat

   
Area-Parity

1.12.2 missing features

  
Area-Rendering

Rendering code, might be shaders specific or general.

  
Good first issue

   
MC-1.19.2

   
MC-1.20.1

   
MC-1.20.4

   
MC-1.20.6

   
MC-1.21

   
MC-1.21.1

   
Modtoberfest

   
P-0-High

High priority, crash/breaking.

  
P-1-Medium

Medium priority. Potentially deleting items or breaking functionality.

  
P-2-Low

Low priority. Not hurting anyone, just annoying.

  
Status-Awaiting Response

Waiting for OP to respond

  
Status-Behind-Flag

This change is available but is placed behind a feature flag.

  
Status-Blocked

Blocked by a specific issue or protocol.

  
Status-Cannot Reproduce

   
Status-Duplicate

This issue or pull request already exists

  
Status-Help Wanted

We need some help to try and track this thing down..

  
Status-Incomplete Report

   
Status-Invalid

This doesn't seem right

  
Status-Needs LTS Backport

Fixed in latest but requires LTS backport.

  
Status-Needs Updating

Needs updating to a newer Minecraft version.

  
Status-Stale

This may not be updated by the OP any further.

  
Status-To Implement

   
Status-Triage

A Triage Member should take a look at this

  
Status-Wontfix

This will not be worked on

  
Status-Wontmerge

This PR won't be merged, but is left now for reference of ongoing or future works.

  
Type-Backport

   
Type-Bug

There is a problem.

  
Type-Documentation

Improvements or additions to documentation

  
Type-Enhancement

New feature or enhancement to existing feature.

  
Type-Question

Further information is requested

  
Type-RFC

Request for comments.

  
Type-Suggestion
No labels
Area-Assets
Area-Backend
Area-Conduits
Area-Datapacks
Area-Lang
Area-Mod Compat
Area-Parity
Area-Rendering
Good first issue
MC-1.19.2
MC-1.20.1
MC-1.20.4
MC-1.20.6
MC-1.21
MC-1.21.1
Modtoberfest
P-0-High
P-1-Medium
P-2-Low
Status-Awaiting Response
Status-Behind-Flag
Status-Blocked
Status-Cannot Reproduce
Status-Duplicate
Status-Help Wanted
Status-Incomplete Report
Status-Invalid
Status-Needs LTS Backport
Status-Needs Updating
Status-Stale
Status-To Implement
Status-Triage
Status-Wontfix
Status-Wontmerge
Type-Backport
Type-Bug
Type-Documentation
Type-Enhancement
Type-Question
Type-RFC
Type-Suggestion
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Team-EnderIO/EnderIO#547
No description provided.
Delete branch "dev/rewrite-xp-logic"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?