Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Placing grass block over grass #12

Open
AppPhil opened this issue Jul 4, 2016 · 31 comments
Open

Placing grass block over grass #12

AppPhil opened this issue Jul 4, 2016 · 31 comments

Comments

@AppPhil
Copy link
Contributor

AppPhil commented Jul 4, 2016

When you place a grass block over another grass block the lower one doesn't gets to normal dirt. But maybe that is planned like this.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 5, 2016

What do you think about adding a method to Block that gets called when a block is placed at the top/bottom/left/right of it?

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 5, 2016

You'll want to look at the Block class, specifically the onPlace method which is called when a block is placed. From that method you can check the tiles around you.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 5, 2016

But is it the best way to just check if the placed block is a grass block and then check if the block under is also a grass block? (everything in onPlayer)

@jordanb84
Copy link
Contributor

jordanb84 commented Jul 7, 2016

Regarding @AppPhil 's last statement, this seems like a function you'd eventually apply to many blocks, so it might be best to write more reusable code for it instead of hard coding each.

One possibility could be a setReplaceNearby(direction, replaceType, newType) function so blocks could set a type which would be replaced by another type if said block is colliding in the specified direction. So for this case, it could be setReplaceNearby(down, grassType, dirtType) in the grass block code which would cause grass blocks under other grass blocks to be replaced with dirt.

This could be used in other cases such as say lava colliding with water. Use this function for each direction, specifying water as the replace type and stone as the new type. Bam, water touching lava is turned into stone with a few simple calls.

You could then expand on it if you wanted, adding the ability to play sound when blocks are replaced, or allow blocks to make the replacements happen on random timed intervals instead of instantly. You could have a slowly spreading block, e.g. maybe grass spreading slowly to nearby dirt or a corrupt poison spreading.

I'm not fully sure how your block system works yet, but I'm sure the general idea can be implemented.
This is of course pointless if the functionality won't be used more than once, but I think it could lead to many interesting uses.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 7, 2016

Good idea, I don't see a problem with adding this functionality. It would still need to be used within onPlace though. This method could lie in the Chunk class and be used when needed for easy replacements like you suggest!

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

When I create a class GrassBlock that extends Block and I use this for the grass block in the BlockManager all the blocks are black under the grass blocks. I don't know why I just got a GrassBlock class looking like this:
public class GrassBlock extends Block { public GrassBlock(BlockType type, Drawable drawable, float lightBlockingAmount, boolean collides, boolean transparent, float initHealth, float resistance) { super(type, drawable, lightBlockingAmount, collides, transparent, initHealth, resistance); } }

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

It looks like this:
removed cause of old textures

Hope you can help me.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Very strange. It must be something to do with initializing the block once it's set in the chunk. I will need to look at the Chunk code to see, but I'm pretty sure the bug is originating somewhere in there.

Edit: I believe it may be coming from here, but I am not able to test it at the moment. If you comment out the code from line 257-259, it may fix this issue, but it will also break torches that are placed when they are loaded from a saved world. If commenting out those lines fixes the issue, we may need to create a separate initialize method in the Block class that will initialize any properties of a Block. In a LightBlock's case, it would set light. This intialize method would only be called when loaded from a saved or newly generated chunk.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

You're right. Commenting out those lines fixes the issue and it breaks torches that are placed. But I'm not sure what you mean how to fix that.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Does it break the torches when you place them in the game, when the world is loaded, or both?

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Only when the world is loaded.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Wait I found out that it happens with the latest version that's here on GitHub so it weren't my changes that did that.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

What do you mean exactly? What part of the problem was happening in the last version?

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

I think the light is broken after loading a world.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Interesting. I will have to take a look, but if I remember accurately it was generating the light correctly. As for the problem on hand however, the code that you commented out was done so that other blocks would have special properties initialized. In this case, the only block that this was applied to was the LightBlock. If you take notice, it was calling the onPlace method which, for a LightBlock, initializes the light on the world. Take a look at the method that this block of code resides in and you'll notice that it is only called when a Chunk is first generated or loaded from within the ChunkManager. Because not every Block needs special values initialized when it first loads, what the code is doing, calling the onPlace method, is not a good idea. Instead what needs to be done is a new method needs to be created in the Block class, onLoad(ChunkManager chunkManager, Chunk chunk, int chunkTileX, int chunkTileY). This method will be called only when a Block is first loaded into a Chunk through either generation or loading from a saved game. The body of it will be empty in the Block class, but in the LightBlock class the body of the onPlace method should be copied into the onLoad method (even better, it should be copied to a separate, private method that is called from onPlace and onLoad). If you read through the LightBlock's onPlace method you'll see that all it's doing is generating light. In order to implement this method in the Chunk, you simply need to replace the three lines you commented out with a call to the Block's onLoad method.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Yes now it works with the latest version from GitHub. I don't know what was wrong there. Sorry for that.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Great. Once it's all ready be sure to submit a pull request!

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Now I've done what you said but after loading the world the torches only light up one block (their position) and not the blocks near them.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Can you commit your code to your forked repository so I can take a look at it?

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Here it is: https://github.com/AppPhil/TerraLegion

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Okay so from that commit it looks like you only created the GrassBlock class. You never actually used it. You also never implemented the onLoad functionality that I was explaining above so the lighting issue won't be fixed. If it's too confusing I will be able to fix it later tonight so it's no problem at all. The reason that the grass block probably isn't replacing blocks under it is because you never actually set the BlockType.GRASS to an instance of the GrassBlock class. In the BlockManager you need to replace the Block instantiation with a GrassBlock.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Wait. That replacing worked for me. I think I have not commited everything. Oh yes wait I will update...

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Strange that the light still doesn't generate correctly. If you want to do some investigative work you can debug from the point that the LightUtils.extinguishLight method is called and figure out why it is only generating light on the one block. I'll try testing it out later tonight as well. Also, make sure that in the BlockManager you create a GrassBlock rather than a Block for the BlockType.GRASS. Once that is done, submit a pull request and we can close this issue.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

Are you sure with LightUtils.extinguishLight? Because that's called in the onBreak method.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 8, 2016

Oh my bad, yes I meant the calculateChunkLight method. My mistake.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 8, 2016

While debugging I found out that the process gets stopped here:

so before the light is correctly set.

In the if-thing the lightValue is 0.95 and posChunk.getLightValue(posChunkX, posChunkY is 1.0

Maybe this does something wrong I don't know:

LightUtils.extinguishLightAmount(chunkManager, realX, realY, lightVal, lightBlockingAmount, lightSource); //update surrounding light values

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 9, 2016

I think it might be not working fine because Chunk.getLightValue() return 1.0 for positions of LightBlocks so the applyLight() method thinks there is already light. But I reallly don't know how to fix that. I tried it for hours today...

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 9, 2016

The lighting algorithm is a very hard algorithm to debug because it's recursive. You have to keep track of what Block it is looking at and it almost requires you to draw it out to understand where you're at. I'll take a look at it later today to see what I can find.

@AppPhil
Copy link
Contributor Author

AppPhil commented Jul 9, 2016

I found out that the light is only not working after loading a world when it is placed in a topChunk.

@jmrapp1
Copy link
Owner

jmrapp1 commented Jul 9, 2016

Interesting. The only different thing, light related, that has to do with the top chunk is that the "sun" lighting is applied. So it may have to be with that. One case that would be an issue is when the lighting is applied, it won't cast it if the light on the block is higher than the light trying to be applied. This is the case you found when debugging. The strange thing is, a block shouldn't have any light value when loaded. The light is applied by the "sun" or torches. Let's continue this conversation in #10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants