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

Reorganize Data (Again) #426

Open
MattEqualsCoder opened this issue Nov 26, 2023 · 2 comments
Open

Reorganize Data (Again) #426

MattEqualsCoder opened this issue Nov 26, 2023 · 2 comments

Comments

@MattEqualsCoder
Copy link
Collaborator

Right now there are like 3 places to edit a lot of the data: the original classes created for generation (Location.cs, Region.cs, etc.), the default C# configs, then the YAML config files setup into profiles.

It would be good to sort of consolidate things a bit more.

A few things to note:

  • We likely have to keep some at least some in the C# code. Primarily, we'd want to keep the logic in the C# code.
  • For everything else, we'd probably want to keep as much as possible in the YAML files I'd imagine.
  • Some things we probably don't want to allow people to override in custom profiles due to potential problems.

My current thought would be to get rid of the default C# configs and move them into default YAML configs that can't be removed by the user. I also think we should apply that metadata when loading directly to the Location/Region/etc. objects instead of having the separate metadata objects we have now. For things we don't want people to override, we can maybe have a DefaultOnly attribute that prevents overriding.

@Vivelin
Copy link
Member

Vivelin commented Nov 27, 2023

Ideally, in my opinion, we'd keep all the hardcoded/least-likely-to-change stuff in code. It's not likely vanilla SM or ALttP is going to suddenly have new item locations, so keeping locations and relevant metadata hardcoded in C# feels appropriate to me.

But stuff that's more likely to change, or that we'd like others to be able to change — alternate names, that sort of thing — stays in config files. I guess that's more or less what we've been doing, but we should revisit that to make sure everything's in the right place.

Either way, the stuff we keep in code should really use a LocationBuilder class or something like that. I could probably chip away at that in my spare time. I also think maybe we should get rid of the strongly typed configuration classes for Tracker's responses since we have so many now. Maybe we just use string keys so it's less of a hassle to add new values?

@Vivelin
Copy link
Member

Vivelin commented Dec 31, 2023

Briefly touched upon this in the Discord chat, but...

The logic for determining what line gets used (at least for tracking items) is kind of getting out of hand. I wanted to add a rare line for picking up missiles that plays regardless of the current item count and without overriding the generic lines, but... that's harder than it seems.

Especially considering the fallback to generic item pickup lines is... quite complicated, and not even part of ItemData or Item, but Tracker.TrackItem itself. And that's quite a convoluted set of if/elses just for determining which line to say.

I don't really have any suggestions; I don't know whether we want to solve this in the YAML by adding more properties to control whether to explicitly include/exclude default lines, or if we want to add a separate "WhenTracked_Always", or something else...

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

No branches or pull requests

2 participants