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

SubLayouts TODO/Wishlist #141

Open
nhmkdev opened this issue Apr 7, 2024 · 28 comments
Open

SubLayouts TODO/Wishlist #141

nhmkdev opened this issue Apr 7, 2024 · 28 comments
Assignees

Comments

@nhmkdev
Copy link
Owner

nhmkdev commented Apr 7, 2024

TODO/wishlist items for consideration

  • Progress Reporting
  • Add metadata to the Deck object for code debugging (knowing the layout name and otherwise in a convenient field could be nice)
    • probably already exists... but nested.
  • [Done] Investigate removal of CardPrintIndex completely. It is likely legacy and should be considered for removal.
    • [low priority] This is really just for the benefit of the code being less complicated
  • [Done] Functionality to export SubLayout to in-memory image for use
  • [Done] Add exportindices sublayout parameter (reuse the export index logic from the command line and export to convert 1-3 etc.)
  • [Done] Reference Cache for files (at least during export)
    • Google cache already handles this (user option)
  • [Done] Add SubLayout parameters
    • skipReferences (don't pass the reference values from parent -> sublayout)
    • skipDefines (don't pass the defines from parent -> sublayout)
  • [Done] Passing custom defines to a SubLayout (as defines via key/value).
  • [Done] Translations:
    • Add fileextension translation as related to exports (to know the file type being exported, probably just pass along the enum value)
    • [Done] Parent layout name
    • [Done] Primary layout name (top)
  • [Done] Investigate the dangers of nested SubLayouts
    • Protect against SubLayout loops
  • [Done] Passing the line information from parent through to all sublayouts in the stack (currently it's just parent -> sublayout each time)
  • [Done] Passing defines through to the SubLayout (not sure how much use this would get)
    • Need to double check if there is special processing for defines that might make this more difficult
  • [Done] FileCardClipboardExporter support (should be minimal)
  • [Done] PDF exporter support
  • [Done] Write instructions (at least the basics) and make a pre-release for people over at BGG to test.

References

Original ticket: #121

@MarkVabulas
Copy link
Contributor

Have you considered changing how all of your column/row calculations are stored/performed, from reading it form the initial spreadsheet, to passing it to the translator, to including extra defines/overrides and then even more SubLayout key/value pairs from the parent?

If all values were parsed per row, then passed around as key-value dictionaries, then it could simplify how the translator works, at the minor cost of rebuilding the dictionaries when changing rows in the spreadsheets. It also means that incorporating the custom _defines, as well as many nested layers of Primary Layouts, becomes trivial, because you just override any existing keys with their new values, then append the rest. Currently you have a more space efficient method, and possibly marginally faster since each row is already in the same order, but it comes at the cost of programmability/extensibility.

The Translators would only care about taking their one dictionary input, and the actual functioning/lookup of them would be effectively identical. It decouples the Translators from the Deck a lot more, and means that passing in arbitrary values with the dictionaries can give easier results (such as filenames). Random string needs to be translated: here's the string and the dictionary and you've got your result. At the moment, that's really hard to do because the keys are stored in one place, in a particular order, and the values are stored elsewhere. Appending them is extremely difficult and has led to the current method of having lookup overrides.

The way I coded the key/value appending originally (before you showed me how it actually works) was to have each PrimaryLayout append/overwrite all of it's values into the SubLayout, and if that SubLayout become its own parent layout, then even the fields passed in from another PrimaryLayout would be passed on in the dictionary to the grandchild SubLayout.

@MarkVabulas
Copy link
Contributor

For protecting against SubLayout cycles, we can keep a running List inside each CardExporterBase (of the name of each PrimaryLayout above it), then if we encounter that the next SubLayout is already in that list we can cancel the cycle.

@MarkVabulas
Copy link
Contributor

I would love to have the functionality for SubLayout in-memory rendering, but then the question becomes how to reference them? Currently, my PrimaryLayout is using a @[ShipType Unique] field in the spreadsheet which declares the name of the suffix of the outputted file from the SubLayout. Then in the SubLayout, it has its own Named ID which I'm using in the Filename override field as Unique_Ship_@[ShipType]. Then in the display of the PrimaryLayout's graphics, it's checking if @[ShipType Unique] is empty, and if it is, then it grabs Normal_Ship_@[ShipType] from the SubLayout, otherwise it grabs the Unique one.

This means that I currently have a SubLayout with 10 Normal_Ship_@[ShipType] in it, that never change except for the colors that are inherited from the ParentLayout, but my SubLayout ALSO has a bunch bunch bunch of other Unique_Ship_@[ShipType] which are variations on the base Ships, but still need the colors/icons from the PrimaryLayout. The colors can change (in the PrimaryLayout), and some of the Unique ships will exist on multiple PrimaryLayout cards, so I don't want to have them as a separate non-SubLayout. Also, I really want to keep consistent sizing/spacing/positioning of the elements between the Unique and the Normal Ship info, so I'm try to avoid having two essentially identical Layouts with identical columns, but different rows, and simultaneously have to make duplicate entries for multiple sets of colors. If I've already got the functionality to duplicate the PrimaryLayout colors to any necessary SubLayout cards, that's what I want.

This hopefully makes it more clear why #noccarddraw would be so useful, because I can check for @[ShipType Unique] inside each SubLayout card, and for only that PrimaryLayout, I can disable the rendering/saving for the Normal or Unique card, as necessary. Also, technically it's not @[ShipType Unique] in the SubLayout, it has a field called @[ShipType] and I'm checking for @[@[ShipType] Unique] when I'm inside the SubLayout (to get which card is correct for the PrimaryLayout).

@MarkVabulas
Copy link
Contributor

As for logging, I think log-only level of reporting for the SubLayout is acceptable, since the SubLayout itself can also be checked as a regular Layout for issues/errors.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 8, 2024

Have you considered changing how all of your column/row calculations are stored/performed, from reading it form the initial spreadsheet, to passing it to the translator, to including extra defines/overrides and then even more SubLayout key/value pairs from the parent?

If all values were parsed per row, then passed around as key-value dictionaries, then it could simplify how the translator works, at the minor cost of rebuilding the dictionaries when changing rows in the spreadsheets.

🤣 I was asking myself the same sorts of questions the other day. I'll have to investigate if there is some critical reason for not using a dictionary for the column mapping to the list of entries in a row. The reference code hasn't changed much in a decade or more so it might be time for me to revisit it a bit.

Edit: did some hacking to remind myself about this... while there are a couple reasons to keep around the raw list<string> there don't appear to be any specific reasons dictionaries can't be used. One interesting case I think I may keep separate is the override columns. I don't think override columns should be passed to sublayouts. That could make a real mess with conflicting element names.

@MarkVabulas
Copy link
Contributor

MarkVabulas commented Apr 8, 2024

10/10 I agree about the override columns. They should be local-presentation overrides and not be transitive :)

Though I've started using #(if @[ImageOffsetX] != #empty then $[x:#math;@[ImageOffsetX]+&[x]#] )# instead of overrides, since generally we just need to move it by a certain amount, instead of a fixed position (with my limited experience), and i find this method works really well since I also keep column names as unique as possible in the majority of my layouts, and certainly between PrimaryLayouts and SubLayouts.

@MarkVabulas
Copy link
Contributor

Would it be possible to get a reference &[fileextension] for the requested file type, accessible from inside a Layout/SubLayout? Then we can swap around file export types and still know which one to reference in the PrimaryLayout, and which ones to check against in SubLayout for #noexport.

@MarkVabulas
Copy link
Contributor

One thing missing from your implementation of the SubLayouts is the FileExportSingleCard version.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 9, 2024

FileExportSingleCard

I'm working on the massive Dictionary change and I held off on FileExportSingleCard as a little bit of logic around initiating sublayouts will be changing too. Hopefully that's not blocking anything too critical for you.

The good news is I was able to have a nested set of sublayouts export with the following nesting

  • PrimaryLayout (provided the title field)
    • SubLayout (was able to access the title field provided from the PrimaryLayout)
      • SubSubLayout (was able to access the title field provided from the PrimaryLayout)

fileextension

I'll add this to the wishlist above for tracking. 👍

Code Refactor: CardPrintIndex Removal

I think CardPrintIndex was use back in the days when there was only one renderer across the entire application. I am going to investigate its removal as part of a future change.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 9, 2024

Comically large and semi-risky change to how the rows store and access their column values has been submitted: b44ce66

FileExportSingleCard should be usable now. I didn't update FileCardClipboardExporter yet (but will at some point).

@MarkVabulas
Copy link
Contributor

So what you're saying is that without CardPrintIndex, and by storing the count of cards in the Dictionary, we can dynamically have card counts work properly with #noexcept? cheeky smile

@MarkVabulas
Copy link
Contributor

From testing the new dictionaries as well as the FileExportSingleCard, they all seem to work as before, so it's cleaner, more functional, and without regression, great job!

@MarkVabulas
Copy link
Contributor

MarkVabulas commented Apr 9, 2024

Wishlist: In addition to &[fileextension] can we add &[currentlayoutname] or something similar which is set to the name of the current PRIMARY layout which is being executed? That way we can add tests for if we're rendering in a sublayout or the primary or most-primary layout.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 10, 2024

WIP commit to protect again sublayout cycles (specifically in file card exports at this time). Bit rough but coming together.

Also added

  • ![rootlayout] (the topmost layout performing the export)
  • ![parentlayout] (the layout requesting this layout export) translators

aa48f01

@MarkVabulas
Copy link
Contributor

To protect against cycles, since you've defined ![rootlayout], you can just check if the layout you're trying to render is ![rootlayout] again and quit?

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 11, 2024

The cycle might start at a lower level. The check I implemented looks at the entire list of layouts (in the "stack" from the root) to make sure the next SubLayout to export isn't already in the list.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 11, 2024

Massive update with hopefully no new issues... image cache is the spot I might expect some problems from. 9570d31

❗ The main point to highlight for those relying on exported file sublayouts is the section below: SubLayouts as In-Memory-Only Exports

SubLayout Elements (Updated)

[layout name]::[parameters]::[define;;value];;[define;;value][repeated as necessary]

The new aspect is there are now two parameters supported:

parameter argument(s) description
nomemexport n/a Provides the ability to turn off memory exports (minor optimization if not needed)
exportformat one of the supported extensions (png, jpeg, bmp...) Enables file writes and indicates the desired format for file exports of SubLayouts

Full examples:

SubSubLayout::exportformat:png::card_count;;10
SubSubLayout::nomemexport;exportformat:png::card_count;;10

Define splitter changed

defines and values are now split with double ;; as ; is likely to come up if a user passes a normal-ish string using a ;

SubLayouts as In-Memory-Only Exports

Now all SubLayouts export into a Bitmap cached in memory by default. They can optionally write to files as well (but you must specify a format).

SubLayouts exports can be accessed via their export name format with no extension.
Layout: SubLayout
Export format: #L_##
Resulting files: SubLayout_1.png (png example if exportformat:png is specified)
Accessing the in-memory image via a Graphic Element: SubLayout_1

These may overwritten with subsequent SubLayout exports (of course).

New variables

variable result
![exporting] 1 if exporting, empty if not
![exportformat] One of the supported file format strings OR empty**

** TODO: will be updated to return empty if only exporting to memory for a sublayout

... hopefully I covered everything, ha!

@MarkVabulas
Copy link
Contributor

I'm really curious about the in-memory method. Do you feel like it will save time avoiding the file round-trip?

I'll check out the performance of all of this too.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 12, 2024

The file write/read for every nested sublayout adds some additional processing time (IO waits etc.).

No need to thrash someone's hard drive unless they really want to. 🔥

@MarkVabulas
Copy link
Contributor

I dropped the folder and .png from all of my SubLayout references, and the memory-based method works amazingly well. I'm a fan. It also seems faster.

@MarkVabulas
Copy link
Contributor

![cardindex] is always 1, now.
#sc;0;1;1# always results in 0

This is in a primary layout with only 1 element, trying to load a graphic based on index.
image

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 16, 2024

Both ![cardindex] and #sc;0;1;1# are related to the count of cards within a single row in the reference. #sc...# is zero based (the first value is the starting number). I'll clarify that in the manual (there's an example but that's not too clear).

![deckindex] is the card number within the entire set of cards. I think this is what you want based on what you're describing.

The ![deckindex] & ![cardindex] were flipped recently as they were backwards.
I may come up with a better name than ![cardindex] for the number of the card within a single row.

@MarkVabulas
Copy link
Contributor

I thought they seemed correct before, deckindex/cardindex, since the cardindex was where we were in the deck, and I thought the deckindex was the index of the layout inside the list of layouts. But thanks, I've made my fix and ![deckindex] definitely works as expected now, thanks

@MarkVabulas
Copy link
Contributor

MarkVabulas commented Apr 18, 2024

I found a bug:

Disabling a SubLayout Element still forces execution of the SubLayout reference, but I think that should only happen if the SubLayout Element is enabled.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 19, 2024

Bug fixed: 24e1412

@MarkVabulas
Copy link
Contributor

MarkVabulas commented Apr 19, 2024

I've seen the light and understand better now one of the ideas/issues you mentioned earlier: I think you're right that having control over which card(s) in the SubLayout run could be a good thing. For example, I want to create a PrimaryLayout which is just a lower-resolution version of a SubLayout, which is itself a PrimaryLayout at extremely high resolution. (Custom control over thumbnails) Doing it as such, there's N^2 iterations of the Main layout.

Changing the definition to [SubLayout Name]::[parameters]::[defines]::[cards-to-execute] could be a pretty good extension. I would end up using ![deckindex] as the [cards-to-execute] part of the SubLayout Element definition, in this case, so that each time the SubLayout is executed, its only the same one. I can, however, imagine needing to combine a few different cards in a row into 1, so having a syntax which allows for something like 1-3 or #math;![deckindex]*3#-#math;2+#math;![deckindex]*3## would be pretty sweet.

@nhmkdev
Copy link
Owner Author

nhmkdev commented Apr 20, 2024

Reference cache enabled during exports (applies to CSV/Excel references automatically): 784886f

This should improve sublayout export speeds and cut down on unnecessary hard drive reads. ... assuming I didn't totally break things. 🤣 🙄

Edit 2: Added exportindices functionality (parameter) for SubLayout definition 5278c2b

SubSubLayout::exportformat:png;exportindices:1-4,8::color1;;0xff0000;;color2;;0x00ff00;;color3;;0x0000ff

New part: exportindices:1-4,8

@MarkVabulas
Copy link
Contributor

Can confirm exportindices works amazingly well. 10/10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants