PacketByteBuf: Rename IntArray to VarIntArray

[Bixilon]

The name IntArray is quite confusing, I am thinking at 4 bytes, not 1-5 bytes.

[apple502j]

In general, a method reading/writing X should always be called readX or writeX. The var-int part is an implementation detail.

[Bixilon]

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.

[apple502j]

@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.)

[Bixilon]

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.

[apple502j]

@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]

🚀 Target branch has been updated to 1.19-pre2

[github-actions]

🚀 Target branch has been updated to 1.19-pre3

[liach]

@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.

[github-actions]

🚀 Target branch has been updated to 1.19-pre4

[Bixilon]

Can we get this before 1.19? (tomorrow)

(then it really might be impactful)

[apple502j]

@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.

[liach]

This isn't really impactful as this is clarifying and there is no fixed-size-int arrays in PacketByteBuf that may cause major confusions.

[github-actions]

🚀 Target branch has been updated to 1.19

[sfPlayer1]

IMO the encoding of the length would be an implementation detail that shouldn't affect the name, but the encoding of each element is very functionally relevant and thus should be reflected in the method name.

[Bixilon]

so...anything here?

[github-actions]

🚀 Target branch has been updated to 1.19.1-pre5

[github-actions]

🚀 Target branch has been updated to 1.19.1