MagicSetEditorPacks / Full-Magic-Pack

Pack containing most all public Magic templates for MSE.
75 stars 17 forks source link

card_fields and mse-includes update #21

Closed G-e-n-e-v-e-n-s-i-S closed 5 months ago

G-e-n-e-v-e-n-s-i-S commented 1 year ago

Add overwritable custom text fields for template makers.

CajunAvenger commented 12 months ago

i'm not entirely convinced by the use case here. in a lot of cases where this can't use an extra card field for some reason, you'd want to know what the field is rather than just "custom 2", and transferring it over to another field later if it becomes "canon" becomes a big headache.

G-e-n-e-v-e-n-s-i-S commented 11 months ago

What you say is true, but it assumes that in those situations you can just add a field in the game files.

If you're willing to do that for me then (I know this is going to sound like a troll but) can you add 6 fields named choice 1 to choice 6 that are defined exactly like those in this pull request?

Alternatively, could you add a default script to level 14 text through level 19 text that's just level_14_text_default() (with level_14_text_default := { "" } in the script file) so I can hook into them?

G-e-n-e-v-e-n-s-i-S commented 9 months ago

I'm proposing a rewrite of the card_fields file. This solves the problem I was having when making this PR, so I'm putting it here. I have changed this PR's title accordingly.

The solution no longer involves custom text fields, but adding a bunch of functions to the default and script parameters of fields, so that templates can override them, giving more control to template creators. For example, in M15 Saga, the default level X fields could probably be replaced by overriding the default function of the level X fields.

I'm also adding a bunch of .mse-includes, in the hopes of automating/streamlining some of the busy work in template creation.

(Unfortunately, Github's diffing algorithm is not very useful when reviewing changes to the card_fields file, since I did a lot of reshuffling of the fields, to group similar ones together.)

CajunAvenger commented 5 months ago

looking over copyright: it looks like we can add extra card fields this way just fine, i'd push for doing that over card fields where relevant, stuff like flavor bars and artist arrows similarly i'd like to look at segmenting these some so all these fields aren't being loaded for templates that have no way to use them anyway

omai the z index arms race

G-e-n-e-v-e-n-s-i-S commented 5 months ago

it looks like we can add extra card fields this way just fine, i'd push for doing that over card fields where relevant

is there a performance reason to that? it seems to me that card fields are always preferable, since you can read their value, and the value transfers over when the user switches templates.

similarly i'd like to look at segmenting these some

definitely the way to go

omai the z index arms race

yeah I just put huge values so there's plenty of space in between. once we have a clearer global picture, we can go back and reel those in.

CajunAvenger commented 5 months ago

is there a performance reason to that?

yes, you eat some performance for each card field, and for each of your extra card fields. if it gets out of hand you can get input lag, it's one of the things that killed the original version of Mainframe that let you do planeswalkers and split cards and masterpiece frames all on a single style, and why mainframes are as economical as they can manage with extra card fields

looking over script rn to get a sense of all the new functions and you actually updated the backwards compatibility flavor bar equation 🫨 that only existed for people too stubborn to use 2.1.2

CajunAvenger commented 5 months ago

if you do need to access the data, then yea card field is the way to go, at least until i get to the MSE 2.4 part of my backlog and we can access those

CajunAvenger commented 5 months ago

big picture thing, i'd consider moving all the include files that operate this way under one .mse-include, something like magic-framebuilding or magic-modules or something. then we can probably standardize the include files magic-modules.mse-include/loyalty/script, loyalty/styling, loyalty/card_field, script_complete, script_dfc sort of setup.

slight con in that versioning the base include file here does become more important, i think this will be an okay trade off in condensing things down to one folder, for knowing where everything is, fewer dependencies to set up, and being easier to move around.

this was going to be a big piece of the 'mainframe rebuild' for 2.0, putting every shared image into an organized magic-mainframe-assets.mse-include file, some of them are already part of this

CajunAvenger commented 5 months ago

whichever way we do it, might be nice to define like copyright_images := "/magic-copyright.mse-include/" module_images := "/magic-modules.mse-include/"

for image: {copyright_images + "art.png"}

CajunAvenger commented 5 months ago

is there a reason that face_coordinates is a string and not an array? as far as i can tell it's entirely backend so it can do arrays instead of converting like we need to do with style options

G-e-n-e-v-e-n-s-i-S commented 5 months ago

is there a reason that face_coordinates is a string

I think it's because at some point I wanted it's value stored in a card field. I don't remember if that's still relevant. Anyway, maybe face_coordinates can be an array, and the card field converts it to a string, rather than the other way around.

moving all the include files that operate this way under one .mse-include

That would be easier in the long run, but how would we handle custom watermark images? People have put them in /magic-watermarks.mse-include/, and now the program would expect them in /magic-modules.mse-include/ or /magic-modules.mse-include/watermarks/

If people still need to put the images in /magic-watermarks.mse-include/ then that negates a big part of the gain.

CajunAvenger commented 5 months ago

custom stuff should go into magic-mainframe-extras whenever possible, iirc watermarks didn't because it would have required magic-watermarks to be a dependent on magic-mainframe-extras which wasn't something i could justify back then.

watermarks as the already established one being an exception is probably fine, pride pinlines probably would too except i don't think anything outside of mainframe used them so we can do that.

alternatively i think pulling off the band-aid is absolutely on the table, anyone who was setting up custom watermarks will be able to handle moving them to magic-mainframe-extras, and it means we can set up a gitignore to gracefully leave custom additions to that file alone and let people fully run MSE off the repo

CajunAvenger commented 5 months ago

also please hold on committing things until my next, i've got a bit chunk inbound

CajunAvenger commented 5 months ago

aight, from here

i'm going to stress test with some big mse files and see if the card fields are too heavy. being able to maintain them across styles and have different sized shapes are nice, but its a pretty small benefit vs hurting render time. one thing that would help is if they don't have scripts/defaults in card_fields, with the scripting happening in choice_images instead; that's something i'll probably look at even if we do keep them

we wanna figure out if there's any additions/changes we want for this draft. find if i mucked anything up.

then make calls on if we're gonna condense, and if we're gonna condense magic-watermarks in as well

G-e-n-e-v-e-n-s-i-S commented 5 months ago

also please hold on committing things until my next

sorry didn't see it in time, hopefully my commit was small enough

i think pulling off the band-aid is absolutely on the table

I'm in favor

CajunAvenger commented 5 months ago

note for future whoever, readme should say where the default position for the offset fields are on a standard 375x523 card

G-e-n-e-v-e-n-s-i-S commented 5 months ago

for watermarks, we can now use

render style: image include file: /magic.mse-game/watermarks/menu_choice_images image: { watermark_scripts[card.watermark](face:1) }

this means that we only have one file to maintain, instead of watermarks watermarks_back watermarks_third who only differ by the face parameter. Also, the render for the previews in the list is less resource intensive.

CajunAvenger commented 5 months ago

k pleasantly surprised with the stress testing, guess i was just using a real bottom of the line computer back in the day.

some notes from using things live

for now i'm going to turn my attention to card fields, reviewing the new fields and scripts there. on your end, please let me know if there's anything at all in the redraft you have issues with, then converting over the other .mse-includes once we have a system we're happy with. for the moment, keep them in their indivual mse-include files

G-e-n-e-v-e-n-s-i-S commented 5 months ago

I see a problem with the face_coordinates_map implementation, specifically this:

fc[input-1] or else fc[0]

it makes it so frames that have only 1 face are actually considered to have 3 faces stacked on top of each other.

I believe the prior behavior was that it would return an array of zeros

G-e-n-e-v-e-n-s-i-S commented 5 months ago

Would a rarity include make sense? Or a complete_typeline include (rarity + type + indicator)?

If so, maybe also a complete_nameline include?

(complete_template include when? 👀)

CajunAvenger commented 5 months ago

yea rarity include is on my shortlist

complete typeline: i wanna see if my concept for auto-shrinking single lines with work first, probably still worth to standardize color identity dot (which we can probably properly automate at this point instead of needing the style option)

complete nameline: if the autoshrink works yea, if not maybe still yea for the styling option instead. once we get into the full madness i have for spree almost certainly

complete template: 🤔

CajunAvenger commented 5 months ago

once we get into the full madness i have for spree almost certainly

i do want to say, my goal for this PR is to get the basic version of this system out the door so we can use it for #18, further modules and more advanced additions can be saved for that or another PR as part of 1.3

i think the aim here is

Changes that require hacking MSE files (like say, moving popout image from image 2 to a dedicated popout image or renaming style fields now that standardization is so free) are on the table, but will need to be a later PR so i can get a tool set up on the LackeyBot website to make those changes to MSE files. the bot already has a system for editing MSE zips so this is fully on the table but will be some extra time that i don't want holding up showcase frames

G-e-n-e-v-e-n-s-i-S commented 5 months ago

I'll make a draft of the nameline package tomorrow.

CajunAvenger commented 5 months ago

auto shrink successful 👏

what we do is 1) adjust the placement of the name field so we can use "middle" instead of "bottom", letting us do away with the manual height adjustment 2) for name and typeline, make a mirror of that field. it has a negative z-index and an arbitrarily wide width, like 2*stylesheet.card_width 3)

mirror_adjust := {
    max_width := 270 ## whatever function here to adjust for node and mana cost
    full_width := card_style.name_mirror.content_width

    if full_width < max_width
        then 1
    else max_width / full_width
}

4) multiply the font size of the regular field with this

at least in the name, this doesn't have the weirdness that rarity and long typelines is causing with printing or exports

G-e-n-e-v-e-n-s-i-S commented 5 months ago

devious

G-e-n-e-v-e-n-s-i-S commented 5 months ago

I found a problem with this :/ Problem is that it's using the card_style variable, which doesn't initialize on templates that are not the default set template until you click on the card.

Steps to reproduce:

This means that before you export a set, you must click on all the cards individually, which is unacceptable. I'll push my implementation here so you can check it out, hopefully I made a blunder.

CajunAvenger commented 5 months ago

name shrink, could try the averaging thing instead (10.5 * length(card.name) ~~ card_style.name.content_width) but the multiplier needs to be figured per unique font and card setup and it'd still need a manual override anyway

potentially having the offset as a save value: true field would get around the initialized problem

G-e-n-e-v-e-n-s-i-S commented 5 months ago

Oh boy, that brings back fond memories of when I was trying to store the text's length inside a custom tag in the Lorcana templates to get around that same exact bug. (It all ended in a garbage fire that's still raging to this day and that I cannot bare to look I'm so sorry nim)

Anyway, VERY interested if you find a solution :)

G-e-n-e-v-e-n-s-i-S commented 5 months ago

Here's essentially what I was doing, if I remember correctly:

get_stored_width := filter_text@(match: "[0-9]+", in_context: "<width=<match>") + to_int
card field:
    type: text
    name: name
    default: "<width=0></width=0>"
    script:
        old_width := get_stored_width(value)
        width :=    if length(remove_tags(value)) == 0 then 0
                else if card_style.name_mirror.content_width > 0 then to_int(card_style.name_mirror.content_width)
                else old_width
        <width=" + width + "></width=" + width + ">" + remove_tags(value)
card field:
    type: text
    name: name mirror
    script: card.name
CajunAvenger commented 5 months ago

hm okay. gives me some more ideas to explore. let's see if any turn out

CajunAvenger commented 5 months ago

losing my mind rn

card field:
    type: text
    name: name mirror
    description: A mirror of the name to help resize it
    show statistics: false
    save value: true
    script: apply_margins(remove_tags(card.name), name:"name_mirror")
card field:
    type: text
    name: pls
    show statistics: false
    save value: true
    script: mirror_adjust(mirror:card.name_mirror, maximum:270)

neither of these fields save??? they don't even save with a bad value they just refuse to save at all

CajunAvenger commented 5 months ago

need to figure out what tomfoolery that's making super type 4 get saved when its not even on the template but not name mirror

CajunAvenger commented 5 months ago

@ more includes

my goal for this PR is to get the basic version of this system out the door so we can use it for #18, further modules and more advanced additions can be saved for that or another PR as part of 1.3

trying to get this approved this weekend and hopefully most or all of the way through showcases, then revisit stuff like cards and crowns after. those may even go to 1.4 where we get battles and the saga PR in

G-e-n-e-v-e-n-s-i-S commented 5 months ago

Yeah sorry, I don't really know what to do now, so I'm just adding images, as these don't really need to be reviewed.

You said a rarity include was on your short list. That can be useful for the showcase frames, but other than that, I believe they have everything they need.

CajunAvenger commented 5 months ago

Yeah sorry, I don't really know what to do now, so I'm just adding images, as these don't really need to be reviewed.

you can open them in another PR, we do need to get to them but i wanna put a solid close line on this rather than let it expand forever and risk letting it keep remaining unfinished

and i will very much be reviewing those images :v the background images should either be full scale textures or cards that are masked, rather than precut

CajunAvenger commented 5 months ago

corners really shouldn't be above overlay stuff, and can be mixed into an updated border handling?

G-e-n-e-v-e-n-s-i-S commented 5 months ago

I'm thinking about corners in a physical way. They represent an absence of cardboard, so in that sense, nothing can go over them.

Also, completely decoupling them from borders is a massive simplification. They can be specified at the set level (which is generally what you want, either all cards have corners or none do), and all we need to do is add the include file: line in older templates without having to worry about how that template's border works, which can sometimes be complicated.

CajunAvenger commented 5 months ago

new sorting is nice, too bad its such a pain to decache the old numbering :pensive:

not sure separators need the set.use_flavor bar call built in. presumably the point of having 8 with a generic name is they aren't all flavor bars?

want to rename copyright b something like secondary copyright rather than b

i don't see the value in card.position; that's easily obtainable by scripts and the default sorting is already by position. appears to be unused as well

similarly not sure if card.face_coordinates does anything

color category, exact color, color identity, color count: guess these can stay if card field bloat isn't a problem anymore. should probably get added to custom_index though

CajunAvenger commented 5 months ago

next up i'm gonna do stamps

for some stuff you can work on if you're itching

G-e-n-e-v-e-n-s-i-S commented 5 months ago
not sure separators need the set.use_flavor bar call built in. presumably the point of having 8 with a generic name is they aren't all flavor bars?

the plan was that they could be used for class levels, case abilities, etc.... not sure how realistic that is though.

* i'd like to see set language moving down when you stack the credit lines as optional behavior with a set or styling option switch

I'll get on that

partition select added to the copyright include, it's a box over the automated card number

just add the partition field over the automated number? should it get disabled if not use_auto_numbers() ?

* `_complete` files should probably get split more-or-less by field

Yeah, my idea was that you would have a rarity include, a color indicator dot include, then the typeline meta-include that groups them. If the typeline_offset functions are defined in the game script file, then the rarity include can reference them, but they can be ignored if you only use the rarity. I don't think defining a lot of variables like that in the script file has any noticeable impact on performance.

i don't see the value in card.position

I had added it when I read this post: https://magicseteditor.boards.net/thread/2494/script-find-card-numbering-cards

apparently when this person was trying to access position(of: card, in: set) it would crash MSE on startup (although it might have been something else they did)

similarly not sure if card.face_coordinates does anything

probably axe it

CajunAvenger commented 5 months ago

Done:

TODO

FUTURE PRs

G-e-n-e-v-e-n-s-i-S commented 5 months ago

For watermarks, I'm a strong proponent of ripping the band-aid now.

For identities, we can move them into modules later. It's gonna take a while to thoroughly modify all existing templates, so it's probably best if we do that in it's own PR.

CajunAvenger commented 5 months ago

i think my plan is to move everything in the repo over, since we can just do that with a find-replace, but leave the old one in place for awhile to give some breathing room for outside templates to switch.

identity-new might just stay indefinitely cause its small but also a pain to update en masse

CajunAvenger commented 5 months ago

abilities_count() >= 5

k this part of loyalty include doesn't play nice with dfcs, this'll either need retooled or this one'll need more a more dedicated dfc alternate include

CajunAvenger commented 5 months ago

with 2.4.0 we can rename set.blend_with_colors to set.default_watermak_render and change it's choices without loss of backwards compatibility

i was thinking of just adding a second switch for the built-in watermarks rather than tying it to what was just meant to be a switch for "will this watermark break if i try to blend it"

G-e-n-e-v-e-n-s-i-S commented 5 months ago

i was thinking of just adding a second switch for the built-in watermarks

Sure, I'll let you see how you want to implement it

pride pinlines

I'm planning on adding a normal pinline include in another PR (#48), so maybe the pride pinline functions should be called pride_pinline_image_ instead of pinline_image_. (Or we can just merge the two includes.)

CajunAvenger commented 5 months ago

ah crap, the single-line shrink effect applies to casting cost, even without scale down to unlike normal text fields, and seems to be ignoring it entirely. gonna tinker a little bit but will probably have to pull that back for now

G-e-n-e-v-e-n-s-i-S commented 5 months ago

Working on the loyalty DFC. Should have it done by tomorrow, however it's a mess and I don't expect it to be useful for Mainframe DFC, nor do I think it's a good idea to make it so.

CajunAvenger commented 5 months ago

k if that just needs to be something i build later we can do that

CajunAvenger commented 5 months ago

idk if there's anything to be done about this, but corners being the highest z-index means popout art can't get fully into the squared corners option image