It is currently 23 Nov 2017, 07:44
   
Text Size

Price Manager issues

Moderators: charmer, CCGHQ Admins

Re: LHpi 2.7/2.8 and howto/documentation

Postby LordHelmchen » 24 Feb 2014, 13:23

anthonybe wrote:From my opinion, the template file should contain the minimal information about properties/functions and their usages (e.g. examples), and the How-to should be a reference where developers can find/share more completed examples (e.g. different scenarios like the foil/regular prices on same page).
Agreed. I will try to keep the template self-documenting, but refrain from adding multiple conflicting example implementations. In return, the wiki can hold examples, including but not limited to, the ones already present in the template.

anthonybe wrote:
LordHelmchen wrote:Please do point out anything that you're unsure about, so I can verify it.
You can find some "TODO" in the how-to for points that i'm not sure about it or I didn't know how to describe in the good way :-)
Sounds good to me.I'll keep on checking for them and resolving them, and you keep on adding them ;-)

anthonybe wrote:For the structure, I tried to make it like a wizard where each required properties are coming first and afterwards optional properties and LHpi properties. At the end, the functions to define.
In my case, I "reformatted" your sitescript template in that order.

If the how-tov and template file have the same structure, developpers can read the how-to and complete their sitescript at the same time. They know what the property/function does and how-to fill it.
I agree on the howto being structured in order of implementation priority, and will leave the ordering to you (as I'm probably biased).
I disagree with reordering the template:
  • Boolean control options are placed in front. Some are potentially needed to be already define for the rest of the sitescript, including function definitions, to execute properly. They also are the only thing the end user should need to edit once the template becomes a released sitescript, so having them accessible at the front makes sense.
  • global configuration elements (like libver,dataver,scriptname) and the global container tables (LHpi and site) need to be defined for the rest of the script to execute properly.
  • site configuration elements (regex,resultregex,currency,encoding) could be moved down and grouped with the tables, if that seems more convenient or logical.
  • running the sitescript with undefined functions will result in LHpi (intentionally and conrolled) throwing an error, while running with empty tables will usually just find nothing to import, so functions were placed above tables due to priority.
Of course, this is open source, and me disagreeing does not mean I'm not grateful for the ideas nor open to further disussion :)
anthonybe wrote:
LordHelmchen wrote:Should the paragraphs from the Hints for developers section stay seperate or be moved over to the howto?
The hint are welcome in the how-to, in the appropriate section.
"multiple copies of the same script" could be added in the "(semi-optional) Name of sitescript".
"procedure of funtions call" could be added in separate section (e.g. annexes ?).
"Dropping card" could be added to the appropriate function definition (ParseHtmlDate, BuildCardData, ...) depending the usage developpers do.
"non trivial tables" could be added as a general tips in the properties introduction.
Will do. I rather like seperating Development (of the library) and Developing (sitescripts for the library) in this way.
anthonybe wrote:
LordHelmchen wrote:Shall I add information about the return values that LHpi expects from the sitescript functions, or are the template comments enough for this?
You're welcome to add these infos in the how-to.
Will do (eventually) ;-) Feel free to start it, adding TODO markers where needed.

Last (for this post, anyway :-P ) remark about the wiki: I'm not documenting changes I make to the wiki in this thread. In this phase of rapid changes to the documentation, I regularly check the wiki's history function and would propose you do the same.

anthonybe wrote:-----------------------
Apart of the wiki page, I have another question about LHpi.
  1. Library updade
    ...
As I'm working towards libver 2.8, I already expected the need for and planned to add "updating sitescripts" sections to the howto, where I will try to document all needed changes.
Until that exists, you can either continue using the library version you're most comfortable with (sitescripts were made to be able to specify and load their preferred library version for this reason), or you may need to use some sort of
diff | Open
are you familiar with this, or does the howto need a section on diff tools as a general hint?
tool to see what is changed and adapt your sitescript. The version history should already give you some small hints on what needs to be adapted, too. Copy/paste from old into new might also work, and potentially prevent the luadoc comments from becoming out of sync (which is a pain to remember updating for all four of my sitescripts each time ;-) ).

In any case, it is probably a very good idea to use ImportPrice from the template version that matches the library version, and only incorporate changes you might have made to it into the the new function definition.

I apologise for the inconvenience this causes.

I will only maintain the template for the latest library version, but library and data versions will stay up in their respective sections as long as there is any current sitescript that depends on that version (and even if not, old versions can still be downloaded via the version history).

anthonybe wrote:
  • Different url infix for same set

    In the last days, the Belgian website did an important update, causing the failure of the my sitescipt.
  • I just love when that happens. See the first two entries in trader-onlineDE version history for my favorite moment so far ;-)
    anthonybe wrote:...
    Do you think it's the correct way ? Or you see another one ?
    2.8 will bring an extended site.frucs format, that better deals with importfoil="n" or "o" cases:
    new site.frucs structure | Open
    Code: Select all
    --[[- table of available rarities.
     can contain url infixes for use in site.BuildUrl

     @field [parent=#site] #table frucs   rarity array { #number = #string } ]]
    --Thre three sample fruc tables below should cover most cases
    site.frucs = {
       [1]= { id=1, name="Foil"   , isfoil=true , isnonfoil=false, url="foil" },
       [2]= { id=2, name="nonFoil"   , isfoil=false, isnonfoil=true , url="regular" },
    }
    --site.frucs = {
    --   [1]= { id=1, name="Foil"   , isfoil=true , isnonfoil=false, url="foil" },
    --   [2]= { id=2, name="Rare"   , isfoil=false, isnonfoil=true , url="rare" },
    --   [3]= { id=3, name="Uncommon", isfoil=false, isnonfoil=true , url="uncommon" },
    --   [4]= { id=4, name="Common"   , isfoil=false, isnonfoil=true , url="common" },
    --   [5]= { id=5, name="Purple"   , isfoil=false, isnonfoil=true , url="timeshifted" },
    --}
    --site.frucs = {
    --   [1]= { id=1, name="nonfoil+Foil"   , isfoil=true , isnonfoil=true, url="" },
    --}
    If the ID of foil and nonfoil pages are somehow related, you could get away with soing some computation between site.sets[sid].url and site.frucs[fid].url fields in your site.BuildUrl.
    Should I hurry and release 2.8 (and continue going to 2.9), so you can use this already?
    2.7 -> 2.8 changes done so far | Open
    Code: Select all
    added savepath writable check to SAVECSV==true in DoImport
    don't assume fruc[1] is foil and all other frucs are nonfoil
       site.fruc: add booleans to have each fruc identify itself as foil and/or nonfoil
       site.BuildUrl: infix changed from site.frucs[frucid] to site.frucs[frucid].url
       ProcessUserParams: disable unwanted frucs in site.sets at runtime, similar to how unwanted langs are already handled
       ListSources: removed check for url.foilonly, ProcessUserParams should have taken care of it.
       BuildCardData: removed @param #boolean urlfoil, as it was not reliable
                      (site.BuildUrl can return the same url twice, with different url.foilonly;
                       only the first of the duplicate urls is kept).
                       If you need to query url.foilonly, do it in site.ParseHtmlData.
       BuildCardData: make sure userParams are honoured, even when preset data is present, by setting unwanted prices nil before returning the card
    plans for either 2.8 or 2.9 | Open
    Code: Select all
    add CHECKSTRICT option below CHECKEXPECTED to toggle notallgood triggering on drop,namereplace,foiltweak; otherwise, CHECKEXPECTED will only care about prices set and failed.

    to anticipate sites that return "*CARDNAME*REGPRICE*FOILPRICE* instead of "*CARDNAME*PRICE*FOILSTATUS*"
     have site.ParseHtml return a collection of cards, similar to site.BuildUrl
    Otherwise ... I think you found the best solution. site.sets[sid].url is not used in the libray, so its safe to define it however you need it to get BuildUrl do what is right.

    anthonybe wrote:(Still a good example to add in the how-to ;-))
    feel free to run wild with it :mrgreen:
    LordHelmchen
     
    Posts: 125
    Joined: 21 Aug 2012, 16:06
    Has thanked: 20 times
    Been thanked: 32 times

    Re: Price Manager issues

    Postby anthonybe » 05 Mar 2014, 10:37

    First of all, sorry for my late response.

    I changed the order my LUA script according to the order of the wiki and did not notice any problem :-)
    I will send you the script by PM, so you can see how is it.

    Last (for this post, anyway :-P ) remark about the wiki: I'm not documenting changes I make to the wiki in this thread. In this phase of rapid changes to the documentation, I regularly check the wiki's history function and would propose you do the same.
    Will do ;-)

    As I'm working towards libver 2.8, I already expected the need for and planned to add "updating sitescripts" sections to the howto, where I will try to document all needed changes.
    Until that exists, you can either continue using the library version you're most comfortable with (sitescripts were made to be able to specify and load their preferred library version for this reason), or you may need to use some sort of tool to see what is changed and adapt your sitescript. The version history should already give you some small hints on what needs to be adapted, too. Copy/paste from old into new might also work, and potentially prevent the luadoc comments from becoming out of sync (which is a pain to remember updating for all four of my sitescripts each time ;-) ).

    In any case, it is probably a very good idea to use ImportPrice from the template version that matches the library version, and only incorporate changes you might have made to it into the the new function definition.
    I use Kdiff3 for thins kind of comparison.
    No problem with that. I will re-create my script with your latest library and do copy/paste.
    Once my script is done, I will keep the library version and do the update only if required.

    Concerning 2.8, no urgence. I will do the solution I proposed.

    Concerning the Library, I have an idea for improvement.
    For now, the template can define site.variants and if it does, it need to define all possible variants (already defined in LHpi) even if template add only one variants.
    It would be great if LHpi could merge the default variants (defined by LHpi) with variants defined by the template, with precedence over template variants.
    In that case, template can define only ONE variant it found usefull and do not copy/paste ALL variants from LHpi, which is a pain to maintain (LHpi <> template).
    anthonybe
     
    Posts: 19
    Joined: 07 Nov 2013, 21:51
    Has thanked: 0 time
    Been thanked: 2 times

    Re: Price Manager issues

    Postby LordHelmchen » 05 Mar 2014, 13:44

    anthonybe wrote:First of all, sorry for my late response.
    I was slightly worried that the latest flurry of changes discouraged you. Glad to see you're still with me.
    I changed the order my LUA script according to the order of the wiki and did not notice any problem :-)
    I will send you the script by PM, so you can see how is it.
    I'll check it out and try to come up with helpful remarks ;-)
    I use Kdiff3 for thins kind of comparison.
    No problem with that. I will re-create my script with your latest library and do copy/paste.
    Once my script is done, I will keep the library version and do the update only if required.

    Concerning 2.8, no urgence. I will do the solution I proposed.
    2.8 is released already, with significant improvement to fruc handling and also some bugfixes.
    lib 2.8 changelog | Open
    Code: Select all
    DoImport: added savepath writable check to SAVECSV=true
        don't assume fruc[1] is foil and all other frucs are nonfoil

            site.fruc: add booleans to have each fruc identify itself as foil and/or nonfoil
            site.BuildUrl: infix changed from site.frucs[frucid] to site.frucs[frucid].url
            ProcessUserParams: disable unwanted frucs in site.sets at runtime, similar to how unwanted langs are already handled
            ListSources: removed check for url.foilonly, ProcessUserParams should have taken care of it.
            BuildCardData: removed @param #boolean urlfoil

        BuildCardData: make sure userParams are honoured, even when preset data is present, by setting unwanted prices nil before returning the card
        BuildCardData: BCDplugins should know everything BCD knows, so we're passing them importfoil, importlangs
        GetSourceData: now expects site.ParseHtmlData to return card(s) wrapped in a container table
        new STRICTCHECKEXPECTED option
        Toutf8: new optional parameter enc
        string.format'ed all nontrivial Log(string)s
        read static language fields from LHpi.Data.languages
        shortened site.langs
        hotfix: two bugs fixed (thanks to Bloodnut for reporting them)
    There's an updated template that incorporates those changes.
    Also, 2.9 is almost ready, with the third bug found by Bloodnut fixed (and should put an end to to this phase of rapid changes).
    lib 2.9 changes | Open
    Code: Select all
    (all changes so far are transparent to sitescripts)
       fixed and improved MergeCardrows (was broken if importfoil=="N" or "O" - reported by Bloodnut)
       got rid of most global variables
          curhtmlnum      local in MainImportCycle
          persetcount    local in MainImportCycle
                      BuildCardData also returns #boolean namereplaced, #boolean foiltweaked
                      SetPrice also returns #table psetcount, #table failcount
          totalcount,setcountdiffers      local in DoImport
                      MainImportCycle returns #table totalcount, #table setcountdiffers
       misc. small improvements to code and/or comments
       fixed loading of data file from deprecated "Prices" location
       fixed SAVETABLE folder writable check
       fixed logging resultregex finds
    lib 2 changes | Open
    Code: Select all
    (all changes transparent to sitescripts)
       minor improvements to some special sets' variant
       misc. small improvements to code and/or comments
       added 636:Salvat 2011
    template 2.9.2 changes | Open
    Code: Select all
    too much simplification prevented possible functionality: reintroduced site.langs[langid].id field for reverse lookup
       split script version from scriptname to ease possible future change to versioning scheme
       misc. small improvements to example code and/or comments
    You may want to prepare your script to be 2.9.2.x ... I apologise for putting you in a moving-goalpost-situation.

    Concerning the middle point of sitescript changes: I have now moved LHpi development to a local git repository, so that should take care of having multiple (dev) versions and identifying which one to use within Magic Album for me. Thus, I think it's better (from an enduser perspective) to remove the version numbers from the sitescripts' filenames. They still need to be defined within the scripts, both for lib/data loading and to identify the version in the logfile, and lib,data and template will still have them in the filename.
    some more rambling about my reasons | Open
    The advantage (for the user) is that the .csv files that MA stores the imported prices in will not accumulate (leading to a cluttered "\Prices") and old prices that are removed from the websites are still available (as the import will not overwrite existing prices with 0). I'm still on the fence if the logfiles chould carry version numbers or not (clutter vs. easy identification and multiple versions in paralell). Additionally, having the version numbers only inside the script removes redundancy and the risk of going out of sync.
    The disadvantage is that I need a few more click to prepare a release to add version numbers to the zipfile, and that an unsuspecting user could accidentally overwrite old files that he wants to keep.

    Of course, this only really affects my sitescripts, so you're free to adapt any versioning and file naming schmeme that fits your needs. The only change is the assurance that the
    Code: Select all
    scriptname = "LHpi.sitescriptTemplate-v" .. libver .. "." .. dataver .. "." .. scriptver .. ".lua"
    is not as vital as is seemed to be :)

    Concerning the Library, I have an idea for improvement.
    For now, the template can define site.variants and if it does, it need to define all possible variants (already defined in LHpi) even if template add only one variants.
    It would be great if LHpi could merge the default variants (defined by LHpi) with variants defined by the template, with precedence over template LHpi.data variants.
    In that case, template can define only ONE variant it found usefull and do not copy/paste ALL variants from LHpi, which is a pain to maintain (LHpi <> template).
    Funny thing you mention that. I was stumbling over that just now while testrunning the new 2.9.2.1 sitescripts (Do I really want to duplicate all basic lands variants for the sake of two soldier tokens?) ...
    I initially decided against it, because I wanted to avoid non-obvious states, so it's either default or sitescript variants table. Further, once you have a variant table for a set that works with your queried site, no further changes should be necessary to that set's variants (unless the site (or the set :-P ) changes). On the other hand, I agree that reducing redundancy/code duplication is the whole point of a library, so I'm open to suggestions on how this should behave. Are we sure that there is no caveat in having unused/unwanted variant definitions from the default tables hanging around after merging? If the same cardname is defined in both LHpi.Data and sitescript, sitescript should take precedence? do we prefer the additional loops and runtime cost the merging needs to the need to replicate the default table? (anything said about variants applies to and will be implemented similarly to foiltweak)
    LordHelmchen
     
    Posts: 125
    Joined: 21 Aug 2012, 16:06
    Has thanked: 20 times
    Been thanked: 32 times

    Re: Price Manager issues

    Postby anthonybe » 05 Mar 2014, 21:10

    Concerning my sitescript and version 2.9, I will wait you release the new library before rewriting my sitescript. For now, I use version 2.7 and the sitescript is broken due to Belgian website refactoring.
    I did not have time to fix the script until now. So waiting the release 2.9 is a good starting point to start again :-)

    Concerning the variants, does it really cost to merge LHpi and sitescript at runtime ?
    I don't know performance of LUA script but it should not cost too much time to do the merge. Does it?
    Concerning duplication of variants definition, do we need to do it or not ? I would say that LHpi should take care of default variants and if the sitescript does not have expected results, it will add needed variants over LHpi. So, the variants priority would be: First use sitescript variants; if sitescript does not have variants for card name, use the one from LHpi if it exists.
    anthonybe
     
    Posts: 19
    Joined: 07 Nov 2013, 21:51
    Has thanked: 0 time
    Been thanked: 2 times

    Re: Price Manager issues

    Postby LordHelmchen » 06 Mar 2014, 01:45

    anthonybe wrote:So waiting the release 2.9 is a good starting point to start again :-)
    2.9 is released, and library development should have stabilized again. hope it works for you. Would you be so kind and delete and/or incorporate into the main text the sections on updating to new sitescript version and the version specific notes, once you no longer need them?
    Concerning the variants,
    Probably not that much of a cost, especially considering how much looping LHpi already does (probably half of them unnecessary or only for sanity checks ;-) ).
    handling of default variant/foiltweak tables is on the list of future improvements, but I think it can wait (probably until BNG NYX triggers another flurry of development). Merging custom (site) and default (data) tables instead of only using data if site is undefined means I will also need to add a trigger to skip merging and hard-override the default tables. (LHpi.mtgmintcard has an example where this will be necessarry once merging is implemented.) Without this trigger, we would need to explicitely nil each unwanted entry seperately, so the maintenance problem would only be reversed.
    LordHelmchen
     
    Posts: 125
    Joined: 21 Aug 2012, 16:06
    Has thanked: 20 times
    Been thanked: 32 times

    Re: Price Manager issues

    Postby LordHelmchen » 07 Mar 2014, 00:30

    turns out the runtime cost is negligible 8)
    performance tests | Open
    dummy.loadscript("LHpi.mtgmintcard.lua",path,savepath)--2.9.2.9
    OFFLINE = true
    SAVELOG = true
    local fakeimportsets = coresets
    ImportPrice( fakeimportfoil, fakeimportlangs, fakeimportsets )

    minimal
    run 1: 6.476 seconds
    run 2: 6.88 seconds
    run 3: 5.499 seconds
    run 4: 5.132 seconds
    run 5: 5.173 seconds
    run 6: 5.185 seconds
    run 7: 5.154 seconds
    run 8: 5.286 seconds
    run 9: 5.039 seconds
    run 10: 5.07 seconds

    full log (VERBOSE,LOGDROPS,LOGNAMEREPLACE,LOGFOILTWEAK,CHECKEXPECTED,STRICTEXPECTED)
    run 1: 7.16 seconds
    run 2: 6.98 seconds
    run 3: 7.07 seconds
    run 4: 7.07 seconds
    run 5: 7.13 seconds
    run 6: 7.15 seconds
    run 7: 7.05 seconds
    run 8: 7.01 seconds
    run 9: 7.05 seconds
    run 10: 7.05 seconds

    full DEBUG (DEBUG,DEBUGFOUND)
    run 1: 94.3 seconds
    run 2: 93.3 seconds
    run 3: 91.5 seconds
    run 4: 92.5 seconds
    run 5: 91.7 seconds
    run 6: 93.5 seconds
    run 7: 93.6 seconds
    run 8: 94.4 seconds
    run 9: 91.3 seconds
    run 10: 92.9 seconds

    new variant,foiltweak merge, full log
    run 1: 7 seconds
    run 2: 6.84 seconds
    run 3: 7.37 seconds
    run 4: 6.98 seconds
    run 5: 6.98 seconds
    run 6: 6.88 seconds
    run 7: 7.05 seconds
    run 8: 6.91 seconds
    run 9: 6.94 seconds
    run 10: 7.36 seconds


    Feature request implemented.
    LordHelmchen
     
    Posts: 125
    Joined: 21 Aug 2012, 16:06
    Has thanked: 20 times
    Been thanked: 32 times

    Re: Price Manager issues

    Postby anthonybe » 07 Mar 2014, 10:08

    Thank you very much for the fast implementation/perf tests.

    I will download the 2.10 version and rewrite sitescript from the template.
    Once working, I will reorder the sitescript to be compliant with wiki order and see if it's still working :-)

    I will keep you informed about that test.
    anthonybe
     
    Posts: 19
    Joined: 07 Nov 2013, 21:51
    Has thanked: 0 time
    Been thanked: 2 times

    Previous

    Return to Magic Album

    Who is online

    Users browsing this forum: No registered users and 4 guests


    Who is online

    In total there are 4 users online :: 0 registered, 0 hidden and 4 guests (based on users active over the past 10 minutes)
    Most users ever online was 279 on 11 Jul 2013, 22:03

    Users browsing this forum: No registered users and 4 guests

    Login Form