Question on TerraFrame.java

[BusyByte]

I've forked and attempting to rewrite in Scala and had some questions.
There are times in TerraFrame.java where you have a for loop which starts at 1 instead of 0.
It appears like this might be a bug and I have been translating it as is but wanted to double check.
It seems like when you are initializing stuff you'd miss the first item.

[raxod502]

Note that item ID 0 corresponds to "empty" and block ID 0 corresponds to "air". Thus there is no need to load textures for them.

[raxod502]

Also, I really hope you're joking ;)

[BusyByte]

Nah, was looking for something to do, not sure if it would go anywhere.
Was also thinking of getting rid of that applet/awt stuff for something which will actually perform like LWJGL or Scala.js Canvas

[raxod502]

:O

Well, if you'd like to give it a shot, I certainly won't stop you. I'm not planning to take this project anywhere, so you somehow manage to turn it into a real game (quite a task, I imagine, given the current state), feel free to distribute or market it as you see fit. Do let me know if you have any other questions.

[BusyByte]

Cool, thanks. I'll let you know if I have anymore questions

[BusyByte]

for (int l=0; l<3; l++) {
            for (y=0; y<size; y++) {
                for (x=0; x<size; x++) {
                    if (random.nextInt(1000) == 0) {
                        if (blocks[1][y][x] == 83) {
                            blocks[1][y][x] = 15;
                        }
                    }
                }
            }
        }

Should it be using the layer variable l instead of the 1 or should we just get rid of the outer l loop?

[raxod502]

Lololol this code makes no sense. I believe trees were only on the primary layer (i.e. layer 1), so the assignment statement should be correct. The int l=0; l<3; l++ loop is garbage and should be removed. (Why did I use a local indexing variable for l but not for literally every other for loop in the codebase??)

As a side note, I think the world generation for this game was actually one of its better points. It currently appears to all be commented out (in World.generate), for reasons beyond my comprehension.

[ghost]

@BusyByte
Stuff I know for sure first:

if (blocks[1][y][x] == 83) {
    blocks[1][y][x] = 15;
}

Is effectively

if (blocks[1][y][x] == treeWithNoBarkBlockCode) {
      blocks[1][y][x] = treeWithBarkBlockCode;
}

So this is just

growBarkOnCurrentBlock();

Why you're working on this code is beyond me. Seems like you're barking up the wrong trees. Kappa

The layer indexing in the loop seems a bit more nebulous to me. My intuition from the code I've touched so far is that layer is similar to the layer notion in real terraria, where' there's a kind of foreground layer that you can get blocked on, and other layers where you pass in front of or behind blocks. So I imagine this is assuming that you can only have tree blocks on this layer, or that if there somehow is a tree off that layer it's fallen from grace and won't get rendered by this code. If my intuition is right about what layers means (I think it'll become clear by the time someone gets to the collision detection logic for moving a player around) then you certainly cannot replace [1] in the block references, it must remain 1 for the implementation to be correct. We wouldn't want consumers of TerrariaClone to deal with an unseemly regression.

I'm free to review prs by the way @BusyByte but I'd rather not experience too many spoilers since I'm enjoying the adventure that is refactoring this code. Definitely down to pair or mob with anyone interested. I'm also always available to commiserate about TerrariaClone related trauma.

(EDIT: There's some more layer related code where the delayer power propogation is handled that serves as another example for how layers can work. If you like I can try to find you the line in the original source. It's a bit hard for me to keep track of where things are... sorry 😿 )