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 guide to dungeon hub races #153

Merged
merged 6 commits into from
Apr 23, 2024

Conversation

catgirlseraid
Copy link
Contributor

data for dungeon hub races - needed for another pr

Copy link
Contributor

@Thunderblade73 Thunderblade73 left a comment

Choose a reason for hiding this comment

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

This json is not future prove. Since the races have different states they are in and adding in stuff later on gets comlicated. So make it instead of Map<String,ParkourJson> to Map<String,Map<String,ParkourJson>> and set the second layer to "Nothing, No return"

@CalMWolfs
Copy link
Collaborator

This json is not future prove. Since the races have different states they are in and adding in stuff later on gets comlicated. So make it instead of Map<String,ParkourJson> to Map<String,Map<String,ParkourJson>> and set the second layer to "Nothing, No return"

While this is true, I dont belive waypoints are useful for any of the races other than the no-return, nothing category so in my opinion it is fine as is

@lunaynx
Copy link
Contributor

lunaynx commented Apr 20, 2024

How are they not useful? Sure, they're technically not necessary to do because you can get the talismans without them, but what if someone wants to complete all the objectives or something?

@ThatGravyBoat
Copy link
Contributor

This json is not future prove. Since the races have different states they are in and adding in stuff later on gets comlicated. So make it instead of Map<String,ParkourJson> to Map<String,Map<String,ParkourJson>> and set the second layer to "Nothing, No return"

There is no reason to do this, overcomplicating the data structure for a theoretical future feature that will likely never happen does not make any sense. Either way in the future if such a feature will be made the names can be suffixed or prefixed for example no_pearls:giant_mushroom and no_abilities:precursor_ruins

@lunaynx
Copy link
Contributor

lunaynx commented Apr 20, 2024

How exactly is it "overcomplicating" to make a perfectly logical data structure? Your argument is ridiculous. You're basically saying "we can do it in a hacky way later, so let's not use a logical structure in the first place".

@Thunderblade73
Copy link
Contributor

We have so many scuffed jsons. I don't want to handle another one. Also there is a defined future where this happens, since the feature requested all not 4 to complete the talisman.

@catgirlseraid
Copy link
Contributor Author

fixed :3

@Thunderblade73
Copy link
Contributor

Could you separate the part "nothing" and "no_return" with a different symbol. E.g ":". Makes it easier to work with

@catgirlseraid
Copy link
Contributor Author

done <3

@hannibal002
Copy link
Owner

Can we please start with a template that contains the structure for all possible races? Then just fill in the currently existing data.

@CalMWolfs
Copy link
Collaborator

Can we please start with a template that contains the structure for all possible races? Then just fill in the currently existing data.

The current format allows for it to work fine with new ones already, and I believe that adding these extra fields will just add clutter that is not needed

@catgirlseraid
Copy link
Contributor Author

Can we please start with a template that contains the structure for all possible races? Then just fill in the currently existing data.

done i think :3

@hannibal002 hannibal002 self-requested a review April 23, 2024 21:44
@hannibal002 hannibal002 merged commit c957928 into hannibal002:main Apr 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants