yarn
yarn copied to clipboard
FabricMC
PacketByteBuf: Rename IntArray to VarIntArray
The name IntArray is quite confusing, I am thinking at 4 bytes, not 1-5 bytes.
Yes, but imho this is a special case. Minecraft can write ints and var ints. If minecraft only used var ints, then it would be obvious and an implmentation detail. We need to make a clear difference between those two, they might break stuff while reversing the protocol/protocol changes.
@Bixilon From the user (usually modder, since Fabric is mostly a modding project) perspective, the method's purpose is more important than what it does internally. Even if the method sent the int list by stringifying and concatenating with a space it should still be writeIntList because the method's purpose is to write an int list. (Implementation details are usually written in the javadoc.)
So, I am mainly using yarn to check the (minecraft) protocol changes between 2 versions. When I see writeIntArray then I immediately check for a real int. After doing that mistake, I need to debug and check, compare, ... That would not be necessary if the name would be clearer on that the method does.
What about writeInt and writeVarInt? Technically that method is used to write an int. The critical difference is in the implementation. The same applies for the list or the array (depite only having 1 method and not method for real ints).
Normally the name should be what the method is used to or what it does. I'd name your example probably writeConcatedList.
@Bixilon writeInt/writeVarInt is named that way only because there are two different methods doing two different things and Java doesn't allow such kinds of overloading (obviously). This time there is only one method. Unless you're checking the raw packet, the information on whether the integer is var or not is useless - and if you're using Yarn for that purpose, javadoc is your friend.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@apple502j: var ints are not simply implementation details. If there's a list of ints like uuid, it's preferable to be written as regular int array given its uniform distribution. var ints are only good if the value is potentially small; if it's uniformly distributed within the range of int/long, regular int or long are more preferable.
Can we get this before 1.19? (tomorrow)
(then it really might be impactful)
@Bixilon Generally, impactful renames should not be done during pre-release/release candidate phase. That said, if others agree we should merge this now, then I suppose it's fine.
This isn't really impactful as this is clarifying and there is no fixed-size-int arrays in PacketByteBuf that may cause major confusions.