The current block API #76
BomBardyGamer
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
The current block API is slightly messy, in that parts of it don't make any sense to be exposed in the API, parts of it are different from each other and don't make sense without backend knowledge, and other parts are just messy. It needs a rework. This discussion outlines my issues with the current setup, and I'm asking for feedback and suggestions on how we can do better.
Problem #1 - internal id and stateId fields
The first issue I have with the current API is that we expose the
id
andstateId
fields.These are constantly varying, very unstable internal network IDs. They mostly exist in the API to help with custom world generation (one specific project in particular), and when you're working with the backend of the server, and they were originally placed there because Minestom exposed them and so I thought I should. The problem is that these fields always change, most of the time on every update, and so should not be relied upon. They also may disappear at any time if Mojang decides it.
In addition, working with these properly requires both network knowledge and backend knowledge, and also, the difference between
id
andstateId
in our context doesn't make sense, unless you understand thatBlock
represents both the block type and the block state in one type, which is, again, backend knowledge, as well as vanilla knowledge.Solution
My current solution is, of course, the most obvious one, which is to remove any trace of them in the API, like has been done in the
api-changes
branch, but I want to make sure everyone agrees this is the right decision before we move forward.Problem #2 - properties API containing traces of internal string-based system
The second issue I have with the current API is that the properties system, in particular, has parts that are inconsistent and don't make sense.
The properties system is meant to be a high-level abstraction over the actual string-based key-value system that it hides, and this checks out, for the most part. However, issues do start arising when you look at how it's implemented.
Firstly,
Property
has atoString
andfromString
, which are for converting property values to and from their string-based variants. These are in the API so that customProperty
implementations can provide their own ways of dealing with them. However, as mentioned, nothing about the internal string-based system makes any sense to be in the public API.Secondly, and the one I really can't figure out what to do with, is that
PropertyHolder
exposes the internal properties map, which isString
toString
, which really doesn't make any sense, especially considering the fact thatget
andset
both use generics andProperty
s and do not mention strings anywhere, andavailableProperties
is a set ofProperty
s. The problem is that I don't know how to replace it.Solution
Whether we want to remove
toString
andfromString
from the API is still up for debate. The argument comes down to how much I want to support custom blocks and custom clients, which is something I want feedback from the community on.For the
properties
map issue, I've came up with a few choices, but I don't know which one is best:Map<Property<*>, Comparable<*>>
, but will require taking more from vanilla codeKryptonBlock
- theMap<String, String>
for internal purposes, and theMap<Property<*>, Comparable<*>>
for the API. The problem is that this will drastically increase the memory footprint.Problem #3 - the properties on Block
The third issue is that most of the properties on
Block
don't make any sense, don't need to be in the API, shouldn't be used by plugins, and are things that I can't even document properly, as I don't even know how some of them work.I think that a lot of these properties are useful, but for the backend, and the reason why they are actually on
Block
in the first place is because they came from the JSON, I didn't think to exclude any of them, and because we prefer to useBlock
overKryptonBlock
on the backend at the moment to support custom implementations, which is an entirely different issue in itself that's for another discussion.Solution
Of course, the obvious solution is to just remove all of the properties that don't make sense to have in the API. The problem is determining which properties are useful and which ones are not, and which ones are internal and which ones are not, which is another discussion point I want to bring up.
Some final words
Hello people! I know it's been a few months, I've been busy working on other projects, and got a bit bored of this for a while. But don't worry, Krypton is coming back, and hopefully, better than ever.
I also realise that I haven't been engaging with the community very well over the past year that the project has existed for, and I've been treating this project more like my own personal hobby, and ignoring you guys' existence. I appreciate each and every one of you who have starred this repository and/or joined the Discord and shown your appreciation for the work I have put in to this project. I want to give you guys something back, something that we can all enjoy, and I feel that even if we don't ever complete the project, we can at least say we enjoyed the experience we got from being in the community and being part of the development process.
Beta Was this translation helpful? Give feedback.
All reactions