ubc-minetest-classroom / minetest_classroom

Repo for Mintest Classroom game
GNU General Public License v3.0
10 stars 6 forks source link

Refactor realm define #149

Closed pauldpickell closed 2 years ago

pauldpickell commented 2 years ago

/realm define should really just be refactored into /realm new with <[xsize] [ysize] [zsize]> replacing [pos1X] [pos1Y] [pos1Z] [pos2X] [pos2Y] [pos2Z] as optional parameters. In other words, I should be able to specify the dimensions of the realm that I want and then have that space allocated.

https://github.com/ubc-minetest-classroom/Minetest_Classroom/blob/e33a144db576f3192853d11d7c155e4673db4106/mods/mc_worldmanager/commands.lua#L415-L449

lukasgolson commented 2 years ago

The realm new feature already performs the requested behaviour as described.

Realm define is a testing command that was implemented temporarily for the old UBC map config to “import” existing map terrain . It will be removed shortly as 1) the schematic feature is now mostly complete. 2) the define command has a sketchy implementation that I’m not 100% confident with.

I’ll try to reimplement the ‘define’ functionality into the ‘new‘ command, following your refactoring suggestions.

On Jul 28, 2022, at 5:16 PM, pauldpickell @.***> wrote:



/realm define should really just be refactored into /realm new with <[xsize] [ysize] [zsize]> replacing [pos1X] [pos1Y] [pos1Z] [pos2X] [pos2Y] [pos2Z] as optional parameters. In other words, I should be able to specify the dimensions of the realm that I want and then have that space allocated.

https://github.com/ubc-minetest-classroom/Minetest_Classroom/blob/e33a144db576f3192853d11d7c155e4673db4106/mods/mc_worldmanager/commands.lua#L415

— Reply to this email directly, view it on GitHubhttps://github.com/ubc-minetest-classroom/Minetest_Classroom/issues/149, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABXNSTRT3CIVKWWENZA4OZLVWMPEBANCNFSM547A7RZQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

pauldpickell commented 2 years ago

I just saw that this is indeed the case. Closing this as a non-issue.