TheTrackerCouncil / SMZ3Randomizer

Tracker presents: a casual standalone version of the Super Metroid & A Link to the Past Crossover Randomizer with built-in automatic item tracking and Twitch integration
https://vivelin.net/projects/smz3
MIT License
25 stars 8 forks source link

Reorganize Data (Again) #426

Open MattEqualsCoder opened 8 months ago

MattEqualsCoder commented 8 months ago

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:

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 commented 8 months ago

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 commented 6 months ago

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...