Skip to content

Add Documentation for RandMap functions#240

Open
Drought-Causer wants to merge 6 commits intoprojectPiki:mainfrom
Drought-Causer:main
Open

Add Documentation for RandMap functions#240
Drought-Causer wants to merge 6 commits intoprojectPiki:mainfrom
Drought-Causer:main

Conversation

@Drought-Causer
Copy link

Added a bunch of documentation for functions and members under Cave::RandMap

Squash merge this please

Copy link
Member

@HeartPiece44 HeartPiece44 left a comment

Choose a reason for hiding this comment

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

Nice! Looks great on the whole, really good to have more documentation in this part of the code. Few nitpick comments (open to being convinced on some of them though).

* @size{0x4}
*/
struct RandMapDraw {
class RandMapDraw {
Copy link
Member

Choose a reason for hiding this comment

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

nit: would keep this consistent with the rest of the codebase and just use struct.

Copy link
Author

Choose a reason for hiding this comment

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

fair enough, I think at some point once we're matching we should go in and make a lot of things classes (at least in JSystem)

* @size{0x4}
*/
struct RandMapChecker {
class RandMapChecker {
Copy link
Member

Choose a reason for hiding this comment

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

also here

void getStartPosition(Vector3f& position, int index);
void getItemDropPosition(Vector3f& position, f32 minDist, f32 maxDist);
void getItemDropPosition(Vector3f* positions, int positionCount, f32 lowerWeightBound, f32 upperWeightBound);
void getItemDropPosition(Vector3f positions[], int positionCount, f32 lowerWeightBound, f32 upperWeightBound);
Copy link
Member

Choose a reason for hiding this comment

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

nit: we don't use array notation for any parameters elsewhere in the codebase, so probably good to keep this in pointer notation - if we want to, we can do a pass through the codebase later and make any lists into array notation to make it clearer later.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I kinda wanted to start the motion towards using list notation, but for now that makes sense. We should definitely consider changing everything to a list in the future because I think it makes function parameters less confusing.

MapNode* getLinkDoorNodeFirst(MapNode*, int, int, int, int&);
MapNode* getLoopEndMapUnit();
u32 getLoopMapNode(MapNode**);
u32 getLoopMapNode(MapNode* nodes[]);
Copy link
Member

Choose a reason for hiding this comment

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

also here and below

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

Successfully merging this pull request may close these issues.

2 participants