48klocs / dim-wish-list-sources

Source files for wish lists for DIM
MIT License
308 stars 98 forks source link

It would be good to have one true source of the picks #18

Closed TimMcJilton closed 3 years ago

TimMcJilton commented 4 years ago

Right now voltron is manually created via lot's of copy-pasta. There are duplicate entries with (possibly) conflicting notes. The source files have changed over time, at the same time the same changes would need to be search replaced in Voltron etc. While this is nice and works well and most likely is in a good state.

It would make sense that Voltron would be an automatically generated file. It would be nice if we had a file called something like FileMeta.json that in there would look something like:

[
  {
      "filename": "panda_sundial_controller.txt",
      "curator": "Pandapaxxy",
  },
  {
      "filename": "panda_black_armory_pve_picks.txt",
      "curator": "Pandapaxxy",
      "pick_type": "PvE",
  },
]

On the first pass, I would be as simple as possible, add notes when they are missing, have a very basic copy-pasta style that we do currently. Most likely that is good enough for our use cases. But it should make voltron easier to maintain since we only need to work withspecific smaller files and then can just trust that voltron just works

We could get fancier with auto-genning notes and de-duping if an item is in multiple lists to make sure the comment reflects it being in multiple lists. Etc. Maybe combine the notes into a single note. But for now it's good for us to know that voltron is accurate to the source material.

48klocs commented 4 years ago

I have nothing against a script to regenerate voltron, but I'm not sure that this is filling any particular need that I have, either.

Wish lists that I generate going forwards are going to carry more involved notes from curators (you can see that in my more recent wish lists), so automatically adding context is probably solved?

I'm also not trying to hug the issue of finding cases where an item is both PvE and PvP (or controller or DPS or any of the other breakdowns), because I get to cheat and put things in the order that I care about (I am not a big PvP player) when I make wish lists,

So. It's nothing I have a pressing need for, but adding a resource to let people generate their own collection files in their forks would be a nice thing to have.

TimMcJilton commented 4 years ago

The biggest reason IMO was just so we know voltron is up to date.

$ wc -l panda_* mercules_*                                                                                        ~/code/dim-wish-list-sources
   4564 panda_black_armory_picks.txt
   2760 panda_black_armory_pve_picks.txt
   1984 panda_black_armory_pvp_picks.txt
    339 panda_dawning_controller.txt
    264 panda_dawning_mkb.txt
     26 panda_dawn_ritual.txt
   3806 panda_forsaken_pve_rolls.txt
   2771 panda_forsaken_pvp_rolls.txt
    269 panda_missing_black_armory_pve.txt
    198 panda_missing_black_armory_pvp.txt
   1330 panda_penumbra_pve.txt
   1000 panda_penumbra_pvp.txt
    485 panda_pit_and_altar.txt
   1572 panda_returning_dawn_controller.txt
   1474 panda_returning_dawn_mkb.txt
   1407 panda_salvation_pve.txt
   1025 panda_salvation_pvp.txt
   1304 panda_season_drifter_pve.txt
    883 panda_season_drifter_pvp.txt
   4205 panda_shadowkeep_pve.txt
   4327 panda_shadowkeep_pvp.txt
   1269 panda_sundial_controller.txt
   1196 panda_sundial_mkb.txt
     34 mercules_altar_of_sorrow.txt
    427 mercules_black_armory_and_sotp_rolls.txt
     21 mercules_black_armory_pulse_bow.txt
    221 mercules_crown_of_sorrow.txt
    124 mercules_dreaming_city_breakdown_rolls.txt
     76 mercules_erentil_and_opulence_iron_banner.txt
    404 mercules_gambit_rolls.txt
    257 mercules_garden_of_salvation.txt
     35 mercules_iron_banner_bonus.txt
     59 mercules_iron_banner.txt
     62 mercules_last_wish_rolls.txt
    125 mercules_menagerie.txt
    315 mercules_osiris_redux.txt
     70 mercules_undying_season_pass_picks.txt
    168 mercules_vex_offensive_breakdown.txt
  40856 total
$ wc -l voltron.txt                                                                                               ~/code/dim-wish-list-sources
36210 voltron.txt

Voltron has 4,000 lines less than the rest of Panda + Mercules (Not including pukes file)

So either a lot of white space has been stripped out... Or there is a file missing here or there.

If you don't have anything against it, the only thing I can't do without your help is to get the ordering you prefer. Otherwise I will do: PvE Panda -> RandomOrderRestOfFilesWithSomeScript -> Trash Rolls

TimMcJilton commented 4 years ago

Hmm actually.. after re-reading the file list. I bet.. 99% that 3,000 of the missing lines are

$ wc -l *controller* 
   339 panda_dawning_controller.txt
  1572 panda_returning_dawn_controller.txt
  1269 panda_sundial_controller.txt
  3180 total

Without those it brings it to a similar number

48klocs commented 4 years ago

Oh, ha! That is very possible - I only rarely hop on the sticks.

TimMcJilton commented 4 years ago

Did a lil PoC of what the input file to read in would look like. Before I go too deep with this.

definitions:
    pandaPvP:
        - panda_black_armory_pvp_picks.txt  
        - panda_missing_black_armory_pvp.txt  
        - panda_salvation_pvp.txt       
        - panda_shadowkeep_pvp.txt
        - panda_forsaken_pvp_rolls.txt     
        - panda_penumbra_pvp.txt 
        - panda_season_drifter_pvp.txt
    pandaNotPvP:
        - panda_black_armory_picks.txt
        - panda_black_armory_pve_picks.txt
        - panda_dawn_ritual.txt
        - panda_forsaken_pve_rolls.txt
        - panda_missing_black_armory_pve.txt
        - panda_penumbra_pve.txt
        - panda_pit_and_altar.txt
        - panda_salvation_pve.txt
        - panda_season_drifter_pve.txt
        - panda_shadowkeep_pve.txt
    pandaMKB:
        - panda_returning_dawn_mkb.txt
        - panda_dawning_mkb.txt
        - panda_sundial_mkb.txt
    pandaController:
        - panda_returning_dawn_controller.txt
        - panda_dawning_controller.txt
        - panda_sundial_controller.txt
    mercules:
        - mercules_altar_of_sorrow.txt
        - mercules_black_armory_and_sotp_rolls.txt
        - mercules_black_armory_pulse_bow.txt
        - mercules_crown_of_sorrow.txt
        - mercules_dreaming_city_breakdown_rolls.txt
        - mercules_erentil_and_opulence_iron_banner.txt
        - mercules_gambit_rolls.txt
        - mercules_garden_of_salvation.txt
        - mercules_iron_banner_bonus.txt
        - mercules_iron_banner.txt
        - mercules_last_wish_rolls.txt
        - mercules_menagerie.txt
        - mercules_osiris_redux.txt
        - mercules_season_of_dawn_season_pass_breakdown.txt
        - mercules_undying_season_pass_picks.txt
        - mercules_vex_offensive_breakdown.txt
    trash_lists:
        - trash_rolls.txt

outputFiles:
    voltron:
        - pandaPvP
        - mercules
        - pandaNotPvP
        - pandaMKB
    voltron_controller:
        - pandaPvP
        - mercules
        - pandaNotPvP
        - pandaController

Inititally I was going JSON , but then I thought "Oh maybe I can use the whole <<: *value with yaml and it's definitions.. but that failed since I am working with lists and not dicts. So it ends up just overwriting eachother. But yaml feels clean anyways as long as we aren't accepting other yaml.

TimMcJilton commented 4 years ago

(BTW totally meant to order it in this order pandaNotPvp then mercules, then pandaPvP, fixed locally)

mattsigal commented 4 years ago

Just wanted to chime in and say that having a voltron-controller would be amazing (as a primarily PS4-based player). Also, out of curiosity, why use the labelling "NotPVP" instead of "PVE"?

TimMcJilton commented 4 years ago

Just wanted to chime in and say that having a voltron-controller would be amazing (as a primarily PS4-based player). Also, out of curiosity, why use the labelling "NotPVP" instead of "PVE"?

The main reason I did NotPvP was because It is PvE + Generic. In reality I could do pandaPvP and pandaPvE and pandaGeneric or something.

mattsigal commented 4 years ago

Cool cool. Personally, just thought NotPVE looked weird, haha.

TimMcJilton commented 4 years ago

Cool cool. Personally, just thought NotPVE looked weird, haha.

Yeah, it does. I'll swap it and start implementing randomly between things at work assuming @48klocs is good with the input file format.

TimMcJilton commented 4 years ago

Will have an initial PR open in a bit. Ran the script by hand and compared it to current voltron

$ python bin/generate_voltrons.py | wc -l                                                                        ~/code/dim-wish-list-sources
37715

$ wc -l voltron.txt                                                                                              ~/code/dim-wish-list-sources
36128 voltron.txt

Is there a good story of how we'd initially verify the regenerated WL? or do you think it will be just fine as long as generally when we compare DIM before//after the new WL look similar for my 400 deep inventory?

TimMcJilton commented 4 years ago

The other option is to have for voltrons:

voltron.txt - manually updated from 48klocs voltron_mkb.txt - Autogen voltron_controller.txt - Autogen anyotherlistofautogennedpossibilities

Added initial pass here #20

48klocs commented 4 years ago

I don't have a terribly great plan in place to verify the transition, unfortunately.

My rough plan: eyeball the output, see that nothing looks too horrifying. Then A/B load the lists and verify that the item count is the same (or bigger-er).

As far as the format goes, I don't super love YAML, but if it's a script that I'm not writing to automate what probably should have been automated before, I'm OK with it.

TimMcJilton commented 4 years ago

It's easy enough to swap to JSON if you prefer it. Main reason I did yaml was because of inheritance. But I'm barely using that just so I can re-use the same title//description in the voltrons.

TimMcJilton commented 4 years ago

Will say, with these bonus files, and the fact that there are so many voltrons. I'm tempted to re-org the directory alittle by adding a folder for pandapaxxy, one for mercules, and then leave voltron in the main directory with the config file.

imax9000 commented 4 years ago

Didn't know about this work before I sent my PR converting wishlists to m4 and generating text files using make in pre-commit hook.

Then, if I understand correctly what you are trying to do here, it could be as simple as having voltron.m4 do a bunch of include(`other_file.m4')

TimMcJilton commented 4 years ago

Didn't know about this work before I sent my PR converting wishlists to m4 and generating text files using make in pre-commit hook.

Then, if I understand correctly what you are trying to do here, it could be as simple as having voltron.m4 do a bunch of include(`other_file.m4') I don't hate the idea. My main focus of this PR is a way to build the primary wishlist file from the source wishlist files rather than manually updating entries across a bunch of wishlist files whenever something is found//changed. Normalize the process a bit.

Right now I'm getting close to converting this script to go just so people can have a compiled version for windows and Linux and then it doesn't matter what version of go etc. they have installed, they just download the bin from github releases. (Was on vacation for a week, and then sick for a week. so just now looking at 48klocs issue running it now)