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

Change FAWE to use an int[] or long[] based ordinal system instead of a char[] ordinal system in order to support more than 65535 different blockstates #1938

Open
JayemCeekay opened this issue Sep 6, 2022 · 5 comments · May be fixed by #2181
Labels
Enhancement New feature or request

Comments

@JayemCeekay
Copy link
Contributor

What feature do you want to see added?

Hello, I am currently working on a Forge adapter update for FastAsyncWorldEdit, and during my work I have come across an issue that prevents me from submitting a pull request for it at this time. FAWE currently uses a char[] based ordinal system when it performs its operations. Due to this, the system is currently limited to supporting a maximum of 65535 blockstates since that is the maximum value of the char primitive data type. As such, I would like to suggest converting the system to be based off of either integer arrays or long arrays. Doing so would allow support for significantly more blockstates added by mods. I have implemented the change to an int[] based ordinal system on my own local version and have my Forge adapter update in a working and nearly complete state.

I would be happy to submit this change myself as a PR if it would be desired.

Are there any alternatives?

Currently there are no alternatives for supporting more that 65535 blockstates in FAWE's ordinal system.

Anything else?

No response

@JayemCeekay JayemCeekay added the Enhancement New feature or request label Sep 6, 2022
@PierreSchwang
Copy link
Member

I'd say you can add the required changes into the Forge PR - As long as the required memory usage isn't that bad. Based on the size, that's roughly 65k times the currently possible array size (and from 1 byte per entry to 4 bytes). Should definitely be respected when finding a solution for that

@SirYwell
Copy link
Member

SirYwell commented Sep 6, 2022

Going above the $2^{31} - 1$ limit of integers would cause far more issues in several places. Therefore, I assume we don't need to consider longs.

For Paper, using int arrays would be mostly wasted memory, so I'd rather have multiple implementations of the relevant parts.
Exposing ints rather than chars in return types or parameters is likely fine, but internal storage should be specialized depending on the amount of block states.
That means we could probably in a first step update the current interfaces (so the usage char is limited to a small scope, like everything based on CharBlocks but nothing else). Then, implementing IntBlocks (and subclasses) should be pretty much straightforward.

Do you have any information what's a typical amount of block states in e.g. mod packs like All The Mods? That would be interesting for context.

@JayemCeekay
Copy link
Contributor Author

According to the output of Block.BLOCK_STATE_REGISTRY.size() there are 625113 blockstates in All the mods 7. When I ran it with Conquest-Reforged, create, and mr.crayfish's furniture mod, I was around 424000 blockstates. Conquest-Reforged adds a lot.

@JayemCeekay
Copy link
Contributor Author

This old PR can be closed, as a new PR has been created with a clean implementation fixing the issue and is created off the newest codebase.

@JayemCeekay
Copy link
Contributor Author

Correction, I thought I was closing the PR, not the issue. Woops!

@JayemCeekay JayemCeekay reopened this Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
3 participants