gnembon / scarpet

Public Repository of scarpet programs for Minecraft
Creative Commons Zero v1.0 Universal
359 stars 161 forks source link

Add single bedrock app #331

Open ysc3839 opened 2 years ago

ysc3839 commented 2 years ago

Find single bedrock on top of the nether. Just like the "Bedrock Ceiling" feature of BoundingBoxOutlineReloaded. Type /single_bedrock to turn on rendering, and type again to turn off. It checks 9x9 chunks around the player on every 20gt. It seems that carpet shapes can be occluded by blocks. (Is there an option to make shapes always visible behind blocks?) So if the boxes are occluded, type /single_bedrock bottom to make it from y=0 to y=127, type again to turn off.

Tested on 1.18.2.

Screenshots:

pseudofractal commented 2 years ago

The app works great, tested it on 1.17.1. I would still like to provide suggestions for some enhancements,

  1. Since this was inspired by BBOR, having a rendering system similar to it would be much more user friendly/short than this one imo. BBOR makes all boxes, boundaries, etc. based on the player's Y level (or it used to at least). So what I am proposing is, check the player's Y position and draw box from there to the y-128 location. With the current system, everyone almost always has to use the bottom sub command if they are below nether roof. Bottom will still retain its use case, but when a player wants something quick, BBOR style of rendering might be easier.

Everything following this are minor nitpicks,

  1. '_tick' runs even if the player is not moving 'on_move' event might come in handy here.
  2. Enabled and Disabled depends on gamerule sendCommandFeedback. Traditional print might be better for servers that have it disabled.
  3. After removing all bedrock from a column, the box still persists. While this does not affect much on a smaller scale, if the draw_radius is increased and someone goes near a mob farm with roof broken in a massive area. I think the large amount of boxes can cause some performance issues.
  4. I don't know enough about task() but I think it would help a lot, if someone increases the draw radius to a big number.
  5. Afaik strict does not really serve any purpose here.
  6. I was confused with all the 'constants' being stored as a variable, looking at chunk_size & block_name specifically.

Regardless of these nitpicks, the app works great with my limited testing.

ysc3839 commented 2 years ago

@SurfingDude-1182 Thank you for your suggestions!

  1. BBOR renders only one block and can be occluded by blocks:
  2. I think on_move would trigger more frequent. It's a good idea to check player's chunk before calculating.
  3. I will change it.
  4. I will change it.
  5. I think using task() would not increasing MSPT, because Scheduled functions run at the end of the tick (link). I will try using task() to see if it's better.
  6. I will change it.
  7. It seems that there's no constants system in carpet script, so I use global variables as constants.
ysc3839 commented 2 years ago

It's very slow when using task(). I set DRAW_RADIUS to 4, with task it takes 2138ms, without task only 73ms.

ysc3839 commented 2 years ago

Updated. Add shapes cache. It will only calculate when the player move to another chunk.

pseudofractal commented 2 years ago

Updated. Add shapes cache. It will only calculate when the player move to another chunk.

Yep, this is exactly what I had in mind.