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

Add single bedrock app #331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ysc3839
Copy link

@ysc3839 ysc3839 commented Jul 21, 2022

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
Copy link

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
Copy link
Author

ysc3839 commented Jul 21, 2022

@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
Copy link
Author

ysc3839 commented Jul 21, 2022

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

@ysc3839
Copy link
Author

ysc3839 commented Jul 22, 2022

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

@Ghoulboy78 Ghoulboy78 added the new-app About adding a new app label Jul 22, 2022
all(rect([x, global_TOP_Y - 1, z], [0, global_CHECK_HEIGHT - 1, 0], [0, 0, 0]), _ != global_BLOCK_NAME),
from = [x, global_TOP_Y - global_DRAW_HEIGHT, z];
if(global_from_bottom, from:1 = 0);
shapes += ['box', global_DURATION, 'from', from, 'to', [x + 1, global_TOP_Y + 1, z + 1], 'color', global_COLOR, 'fill', global_COLOR];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, you can add 'player', global_player so it only goes to that player.

@pseudofractal
Copy link

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

Yep, this is exactly what I had in mind.

Copy link
Contributor

@opsaaaaa opsaaaaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it and look at the code.
It looks good! and It works well.

Wonderful app, Looking forward to using it while playing.

Copy link
Collaborator

@altrisi altrisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-app About adding a new app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants