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.

Sign up for GitHub

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

Jump to bottom

Add vault generation details to seed catalog #729

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

Add vault generation details to seed catalog #729

wants to merge 1 commit into from

Conversation

flend
Copy link
Collaborator

@flend flend commented Nov 19, 2024 •
edited
Loading

Adds vault generation detail to seed catalog. Information is generally interesting and I use it for vault frequency analysis when trying to balance vault generation probabilities.

Invoked as:

 ./brogue --variant "rapid_brogue" --vaults --print-seed-catalog 1 10 10

Typical output is as follows:

        A vault 1 (Outsourced item -- same item possibilities as in the good permanent item reward room (plus charms), but directly adopted by 1-2 key machines.) [type 8]
        > Guarded by a vault 2 (Mud pit -- key on an altar, room full of mud, take key to cause bog monsters to spawn in the mud) [type 51]
        A vault 3 (Kennel -- allies locked in cages in an open room; choose one or two to unlock and take with you.) [type 10]
        > Guarded by a vault 4 (Nested item library -- can check one item out at a time, and one is a disposable key to another reward room) [type 26]
        >> Guarded by a vault 5 (Guardian obstacle -- a guardian is in the door on a glyph, with other glyphs scattered around.) [type 25]
        A vault 6 (Paralysis trap -- hidden pressure plate with a few vents nearby.) [type 68]
        A vault 7 (Remnant -- carpet surrounded by ash and with some statues) [type 63]
  • CSV format picks up a 'kind id' for vaults and items - this again helps creating statistics.
  • Removed 'minimumAltarLevel' which seemed an unnecessary variant-specific constant for a performance speed up only
  • Some whitespace / formatting fixes around areas that were changed

@flend flend force-pushed the feature/include_vaults_in_seed_catalog_v1_14 branch 7 times, most recently from caeef50 to 7fd1e3b Compare November 24, 2024 10:33
@flend flend force-pushed the feature/include_vaults_in_seed_catalog_v1_14 branch 3 times, most recently from 895f662 to 4bd4409 Compare December 15, 2024 17:45
@flend flend force-pushed the feature/include_vaults_in_seed_catalog_v1_14 branch from 4bd4409 to 926c42d Compare December 15, 2024 17:53
@flend flend marked this pull request as ready for review December 15, 2024 18:00
tmewett
tmewett requested changes Mar 6, 2025
View reviewed changes
Copy link
Owner

@tmewett tmewett left a comment

Choose a reason for hiding this comment

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

thanks, seems good, i'd mainly like some docs describing the architecture of the feature

idle thought: damn there's so much boilerplate with linked lists, i wonder if we should move towards reallocing arrays..

src/platform/main.c
@@ -14,7 +14,7 @@ boolean serverMode = false;
boolean nonInteractivePlayback = false;
boolean hasGraphics = false;
enum graphicsModes graphicsMode = TEXT_GRAPHICS;
boolean isCsvFormat = false;
boolean isCsvFormat, includeVaults = false;
Copy link
Owner

Choose a reason for hiding this comment

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

I believe this leaves isCsvFormat undefined - please change to

Suggested change
boolean isCsvFormat, includeVaults = false;
boolean isCsvFormat = false;
boolean includeVaults = false;

src/brogue/Rogue.h
@@ -1392,6 +1392,15 @@ typedef struct keyLocationProfile {
boolean disposableHere;
} keyLocationProfile;

typedef struct machineInfo {
Copy link
Owner

Choose a reason for hiding this comment

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

request: could you add some comments describing what this type represents? e.g. what a child machine is?

src/brogue/Architect.c
Comment on lines +1039 to +1042
void addMachineInfoToChain(machineInfo *theInfo, machineInfo *theChain) {
theInfo->nextMachineInfo = theChain->nextMachineInfo;
theChain->nextMachineInfo = theInfo;
}
Copy link
Owner

Choose a reason for hiding this comment

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

am i right that this seems to add to the second list position? why's that?

src/brogue/Architect.c
return prev;
}

void addMachineInfoAndChainToChain(machineInfo *theInfo, machineInfo *theChain) {
Copy link
Owner

@tmewett tmewett Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

request: This name was not obvious to me at first, could it be improved?

an idea:

Suggested change
void addMachineInfoAndChainToChain(machineInfo *theInfo, machineInfo *theChain) {
void concatMachineInfoChainOntoChain(machineInfo *theInfo, machineInfo *theChain) {

Copy link
Owner

Choose a reason for hiding this comment

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

or maybe something involving the word "prepend"

src/brogue/Architect.c
short originX, short originY,
unsigned long requiredMachineFlags,
item *adoptiveItem,
item *parentSpawnedItems[MACHINES_BUFFER_LENGTH],
creature *parentSpawnedMonsters[MACHINES_BUFFER_LENGTH]) {
creature *parentSpawnedMonsters[MACHINES_BUFFER_LENGTH],
machineInfo *thisMachineInfoChain) {
Copy link
Owner

Choose a reason for hiding this comment

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

can you document what this new parameter does?

src/brogue/RogueMain.c
free(levelMachineInfo);

if (firstRealNode != NULL) {
machineInfo *reversed = reverseAllMachineInfo(firstRealNode);
Copy link
Owner

Choose a reason for hiding this comment

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

why does the chain need reversing? why is the chain order important? if it is, can you document it in the docs for the struct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmewett tmewett tmewett requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@flend @tmewett