MightyPirates / TIS-3D

TIS-100 inspired low-tech computing in Minecraft.
https://www.curseforge.com/minecraft/mc-mods/tis-3d
Other
105 stars 35 forks source link

Display module docs issue #144

Closed crazyStewie closed 2 years ago

crazyStewie commented 2 years ago

The in-game manual implies that the display module has a resolution of 28x28. However, from both testing within the game and looking at the source code, the actual resolution seems to be 24x24. Relevant code : https://github.com/MightyPirates/TIS-3D/blob/ea4c761a770fd82fd7aa81dd7a3651d2a6da5140/src/main/java/li/cil/tis3d/common/module/DisplayModule.java#L77

Relevant docs : https://github.com/MightyPirates/TIS-3D/blob/ea4c761a770fd82fd7aa81dd7a3651d2a6da5140/src/main/resources/assets/tis3d/doc/en_us/item/display_module.md#L9

duncanwebb commented 2 years ago

Don't think that you are correct

2021-10-25_14 33 20

Hard to see the first bar as it is black, but the other bars are clearly longer than 28

crazyStewie commented 2 years ago

@duncanwebb Which version are you using? This is the result I get using your code on the latest avaliable file (tis3d-1.17.1-forge-1.6.6+6.jar from Sep 18, 2021) :

Display result ![image](https://user-images.githubusercontent.com/30158085/138784185-0bc58cc9-6a5f-4285-89ee-3243edd2e481.png)
Code used ![image](https://user-images.githubusercontent.com/30158085/138784621-c1d64d18-4041-41bb-8090-6faa68dd4844.png)

From what I can see, the resolution was changed in b8c6dbb6b3be86621741658022544445c23687c2, while the manual line that refers to it was kept the same since it was created in e2d86a0d66746b7a64259a094332dfa7067f567c

duncanwebb commented 2 years ago

I'm playing all of fabric 3 TIS-3D-MC1.16.2-Fabric-1.6.1.23 TIS-3D-Additions-0.2.1+1.16.2

It looks like a change that would break old code that uses the display module. Honestly, I can't envisage any useful applications for the display module, as it far too slow to be useful and would take a lot of code, five lines of the available 40 for each draw. The best use I can think of is a progress bar. I don't like being negative, as I know how much effort goes into writing reliable code.

Lgmrszd commented 2 years ago

Can confirm, on latest Forge version (tis3d-1.16.5-forge-1.6.6+25) the display size is 24x24

esotericist commented 2 years ago

so interestingly, it looks like before #b8c6dbb the internal resolution was 32x32, rather than 28x28.

fnuecke commented 2 years ago

IIRC I noticed this in the 1.16 port and kind of forgot about adjusting this on the side. The module was always intended to only use the "inner" area of the module (to match all other modules), but it didn't. Fixed the code, but forgot the documentation. Fixed the documentation now.