Changes to Support Worldgen in Custom Dimensions

[outercloudstudio]

Changes

• Added setBlockVolume to LevelChunk
• Implemented BlockVolume
• Implemented buffer_span_mut

Fixes

• Fixed some fields in LevelChunk

[FloppyDolphin57]

There is no deletion implemented for buffer_span_mut. Memory leak!!!

[FloppyDolphin57]

Also setBlockVolume can take in a const BlockVolume& and the second parameter is yOffset

[ZeroMemes]

Ummm put it in a review buddy!!!!

[FloppyDolphin57]

Ummm put it in a review buddy!!!!

Never heard of it

[SaturnRoccat]

There is no deletion implemented for buffer_span_mut. Memory leak!!!

is it actually a memory leak? Its only meant to provide a non owning look at a segment of memory meaning no dtor is fine iirc

[FloppyDolphin57]

There is no deletion implemented for buffer_span_mut. Memory leak!!!

is it actually a memory leak? Its only meant to provide a non owning look at a segment of memory meaning no dtor is fine iirc

this implementation IS owning

[ZeroMemes]

is it actually a memory leak? Its only meant to provide a non owning look at a segment of memory meaning no dtor is fine iirc

while this is true of memory spans, this PR adds a constructor to the span struct that incorrectly allocates a new block of owned memory.

[FloppyDolphin57]

basically time for

template <typename T>
class buffer_span_mut : std::span<T> {};

or using

[SaturnRoccat]

basically time for

template <typename T>
class buffer_span_mut : std::span<T> {};

or using

yeah i just looked at the PR and it seems like that would make the most sense

[outercloudstudio]

Okay I agree with buffer_span_mut not owning the memory as that is what I originally tried, but then I couldn't tell who owned the memory otherwise. It looks like it is just allocated with the BlockVolume.

[FloppyDolphin57]

One last thing, LevelChunk::setBlockVolume should be using member function pointer calling

[outercloudstudio]

LevelChunk::setBlockVolume should be using member function pointer calling

What does this mean lol? I'm still new to c++

[FloppyDolphin57]

LevelChunk::setBlockVolume should be using member function pointer calling

What does this mean lol? I'm still new to c++

How its currently being called is in the same fashion as how a static function would be called. Found some random commented-out functions using member function pointer calling here https://github.com/FrederoxDev/Amethyst/blob/main/AmethystAPI/src/minecraft/src/common/world/level/BlockSource.cpp

[SaturnRoccat]

LevelChunk::setBlockVolume should be using member function pointer calling

What does this mean lol? I'm still new to c++

How its currently being called is in the same fashion as how a static function would be called. Found some random commented-out functions using member function pointer calling here https://github.com/FrederoxDev/Amethyst/blob/main/AmethystAPI/src/minecraft/src/common/world/level/BlockSource.cpp

What does this actually change from the usual method of just making a function that takes the this pointer at the start?

[ZeroMemes]

There’s no functional difference for this specific case, both will compile to the same code in an optimized build. It does matter when a function’s return value requires a stack allocated buffer, so you might as well use the syntax everywhere for consistency.

[FloppyDolphin57]

LevelChunk::setBlockVolume should be using member function pointer calling

What does this mean lol? I'm still new to c++

How its currently being called is in the same fashion as how a static function would be called. Found some random commented-out functions using member function pointer calling here https://github.com/FrederoxDev/Amethyst/blob/main/AmethystAPI/src/minecraft/src/common/world/level/BlockSource.cpp

What does this actually change from the usual method of just making a function that takes the this pointer at the start?

So say I have function Color Class::getDaColor(uint32_t index) and static Color Class::getDaColor(uint32_t index)
these will be called as Color* Class::getDaColor(this_ptr, Color* result, uint32_t index) and Color* Class::getDaColor(Color* result, uint32_t index)

so if I did using function = Color(__thiscall*)(Class*, uint32_t) it would turn into Color* Class::getDaColor(Color* result, this_ptr, uint32_t index) you would really want Color(__thiscall Class::*)(uint32_t);

hope this kinda maybe makes sense lol

[outercloudstudio]

LevelChunk::setBlockVolume should be using member function pointer calling

What does this mean lol? I'm still new to c++

How its currently being called is in the same fashion as how a static function would be called. Found some random commented-out functions using member function pointer calling here https://github.com/FrederoxDev/Amethyst/blob/main/AmethystAPI/src/minecraft/src/common/world/level/BlockSource.cpp

What does this actually change from the usual method of just making a function that takes the this pointer at the start?

So say I have function Color Class::getDaColor(uint32_t index) and static Color Class::getDaColor(uint32_t index) these will be called as Color* Class::getDaColor(this_ptr, Color* result, uint32_t index) and Color* Class::getDaColor(Color* result, uint32_t index)

so if I did using function = Color(__thiscall*)(Class*, uint32_t) it would turn into Color* Class::getDaColor(Color* result, this_ptr, uint32_t index) you would really want Color(__thiscall Class::*)(uint32_t);

hope this kinda maybe makes sense lol

I'll figure out what this means eventually, but why specifically setBlockVolume? Why is _createBlockEntity different?

[FloppyDolphin57]

LevelChunk::setBlockVolume should be using member function pointer calling

What does this mean lol? I'm still new to c++

How its currently being called is in the same fashion as how a static function would be called. Found some random commented-out functions using member function pointer calling here https://github.com/FrederoxDev/Amethyst/blob/main/AmethystAPI/src/minecraft/src/common/world/level/BlockSource.cpp

What does this actually change from the usual method of just making a function that takes the this pointer at the start?

So say I have function Color Class::getDaColor(uint32_t index) and static Color Class::getDaColor(uint32_t index) these will be called as Color* Class::getDaColor(this_ptr, Color* result, uint32_t index) and Color* Class::getDaColor(Color* result, uint32_t index)
so if I did using function = Color(__thiscall*)(Class*, uint32_t) it would turn into Color* Class::getDaColor(Color* result, this_ptr, uint32_t index) you would really want Color(__thiscall Class::*)(uint32_t);
hope this kinda maybe makes sense lol

I'll figure this out eventually, but why specifically setBlockVolume? Why is getBlockEntity different?

As Brady said, it makes no difference here as it's a void return type. But it's good to have consistency throughout the project.

[outercloudstudio]

Also @FloppyDolphin57 Do you know what owns the BlockVolume memory if the span is non owning?

[FloppyDolphin57]

Also @FloppyDolphin57 Do you know what owns the BlockVolume memory if the span is non owning?

I don't think that's something that really needs to be worried about, try just using an std::span it will probably work fine.

[ZeroMemes]

I don't think that's something that really needs to be worried about, try just using an std::span it will probably work fine.

The issue is that the BlockVolume constructor actually fills the span with the default block lol. Something needs to own that memory.

[FloppyDolphin57]

I don't think that's something that really needs to be worried about, try just using an std::span it will probably work fine.

The issue is that the BlockVolume constructor actually fills the span with the default block lol. Something needs to own that memory.

Time to make a vector and pass begin and end to it

[FloppyDolphin57]

I don't think that's something that really needs to be worried about, try just using an std::span it will probably work fine.

The issue is that the BlockVolume constructor actually fills the span with the default block lol. Something needs to own that memory.

Time to make a vector and pass begin and end to it

I think this is really what the game is doing tbh, looking at game calling function with this span type it seems to always be constructed from some array or other type somewhere else

[outercloudstudio]

I don't think that's something that really needs to be worried about, try just using an std::span it will probably work fine.

The issue is that the BlockVolume constructor actually fills the span with the default block lol. Something needs to own that memory.

Time to make a vector and pass begin and end to it

I was doing that earlier. But I can't find where it stores the vector in the game code. To me it just looks like memory is allocated.

[ZeroMemes]

The allocation usually happens on a per-usage basis, sometimes it's a stack allocated array, sometimes it's a stack allocated vector that's resized to the required capacity.

Worldgen does it a little differently though:

// ?loadChunk@OverworldGenerator@@UEAAXAEAVLevelChunk@@_N@Z:
...
v5 = Bedrock::Threading::InstancedThreadLocal<OverworldGenerator::ThreadData,std::allocator<OverworldGenerator::ThreadData>>::_load((char *)this + 432);
...
v56[0] = (char *)v5 + 6144;
v56[1] = (char *)v5 + 661504;
BlockVolume::BlockVolume((unsigned int)v68, (unsigned int)v56, 16, 320, 16, (__int64)RenderBlock, (_DWORD)RequestId);

The buffer is owned by a thread local OverworldGenerator::ThreadData instance, and other dimensions have their respective ThreadData structs.

[outercloudstudio]

@FloppyDolphin57 Were these the changes you meant?

[ZeroMemes]

The actual game function uses a reference not a pointer in LevelChunk::setBlockVolume. First parameter should be const BlockVolume&. (Pointers can be null, references cannot)

Edit: meant to attach this to the relevant line. whoops.