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

Can't edit platforms.json #20

Open
s1eve-mcdichae1 opened this issue Apr 19, 2023 · 4 comments
Open

Can't edit platforms.json #20

s1eve-mcdichae1 opened this issue Apr 19, 2023 · 4 comments

Comments

@s1eve-mcdichae1
Copy link

s1eve-mcdichae1 commented Apr 19, 2023

I mean...I "can", but when-if-ever I update, I have two choices: either it gets overwritten, and I lose my edits or, on the other hand, suppose I don't update that file. Well, then I get to keep my own edits, but I miss out on any new additions from upstream.

(I suppose the same applies to screenscraper.json too.)

I think there ought to be a "system" copy and a separate, optional, non-updating user-configurable "extra" copy of each these files, so we can add our edits and receive updates, without sacrificing either.

@Gemba
Copy link

Gemba commented Apr 24, 2023

I assume the scriptmodule does more than needed here

This line may also be overwriting these existing json files.

Skyscraper does not overwrite existing files when looking here.

@s1eve-mcdichae1
Copy link
Author

I am self-taught and still learning as I go so there's a chance that any or all of this is wrong, but here's what I see when I look at this:

I assume the scriptmodule does more than needed here

For I can tell, these lines retain the two files from the remote repository (the $md_build folder) to the local install folder ($md_inst). These are "master" read-only backup-install copies, not the "live" versions actively used by the application in situ. This is necessary so the files can for to be available in the next step.

This line may also be overwriting these existing json files.

This line copies those read-only install-backup copies from the $md_inst to the "live" versions in the $scraper_conf_dir directory ([1], or by symlink, [2]) that are actively used by the application.

Indeed, this does overwrites an existing file in the same location (for this is necessary in order to obtain updates, else, upstream changes would not be propagated and the local platforms list would merely stagnate.)

As the active copy, this also is where I would make my custom edits.

Skyscraper does not overwrite existing files when looking here.

I actually don't understand what this part is supposed to be doing, here. I don't know if we (RetroPie) even use this part, or if maybe it's bypassed by our install module? I see that it says it won't overwrite an existing file, but I don't even have the /usr/local/etc/skyscraper directory that it's referencing, on my system. I have all of these same files*, but they are in the $scraper_conf_dir mentioned above, and my /usr/local/etc is empty.

*(I do not have a (non-example) import/definitions.dat [3] until I put my own, just this last weekend.)

In any case, the module could be written to behave the same way, and ignore that file if it already exists. The problem I'm envisioning with that is a situation like this:

Upstream repo platforms.json contains systems "foo," "bar," and "baz":

{
    "platforms":
    [
        {name:"foo"}
        {name:"bar"}
        {name:"baz"}
    ]
}

Then, I add my own custom platform "grue":

{
    "platforms":
    [
        {name:"foo"}
        {name:"bar"}
        {name:"baz"}
        {name:"grue"}
    ]
}

...this is where I'm at right now, with a custom system edited in to my "active" copy of the file at /opt/retropie/configs/all/skyscraper/platforms.cfg, that is not in the "master" copy from upstream, stored at /opt/retropie/supplementary/skyscraper/platforms.cfg.

So now, sometime in the future, say upstream adds a different new system "bleen":

{
    "platforms":
    [
        {name:"foo"}
        {name:"bar"}
        {name:"baz"}
        {name:"bleen"}
    ]
}

To get the new "bleen" system, I need to overwrite the file with the new upstream version. But at the same time, to retain my custom "grue" system, I need to not overwrite the file.

It seems to me, that in order to have both "grue" and "bleen," I need a "merged" file that combines the upstream additions with my own edits:

{
    "platforms":
    [
        {name:"foo"}
        {name:"bar"}
        {name:"baz"}
        {name:"bleen"}
        {name:"grue"}
    ]
}

(I have to manually maintain this, re-inserting my custom system whenever an update changes the upstream version)

Or, a separate file for my customizations:

File platforms-default.cfg:

{
    "platforms":
    [
        {name:"foo"}
        {name:"bar"}
        {name:"baz"}
        {name:"bleen"}
        {name:"anything else they ever add upstream, without affecting my custom edits"}
    ]
}

And file platforms-user.cfg:

{
    "platforms":
    [
        {name:"grue"}
        {name:"baz, but custom"}
        {name:"anything else I ever add custom, without affecting upstream edits"}
    ]
}

(I wouldn't need to maintain anything, except when I wanted to alter my local edits. Upstream changes would happen automatically with a package update, and would co-exist peacefully with my custom edits.)

If I am misunderstanding, then how else can I obtain the new systems added upstream, and while retain my own custom additions?

@Gemba
Copy link

Gemba commented Apr 25, 2023

Hi @s1eve-mcdichae1

You might want to have a try with PR #24.

Thanks for challenging my assumptions:

Skyscraper does not overwrite existing files when looking here.

I actually don't understand what this part is supposed to be doing, here. I don't know if we (RetroPie) even use this part, or

This code segment is actually not used in the retropie context, you are right as make install is never called, files never land in /usr/local/etc.

I assume the scriptmodule does more than needed here

For I can tell, these lines retain the two files from the remote repository (the $md_build folder) to the local install folder ($md_inst). These are "master" read-only backup-install copies, not the "live" versions actively used by the application in situ. This is necessary so the files can for to be available in the next step.

In the install_<module> function the md_ret_files=... defines which files must be copied to the $md_inst $from $md_build. And they are copied regardless of existing files in $md_inst.

This line may also be overwriting these existing json files.

This line copies those read-only install-backup copies from the $md_inst to the "live" versions in the $scraper_conf_dir

Yes. Right. However, I changed it in the PR to use $md_build as source, as $md_inst should only be for binaries and some other files, while config should go to $scraoer_conf_dir.

TL;DR: There were two causes which overwrite a customized platforms.json and screenscraper.json

The changes in the PR will put any new files from this repo with the suffix .rp-dist (this suffix is the RetroPie way) alongside to the file with any user made. Thus you will end up with a platforms.json and platforms.json.rp-dist.

When PR #22 also gets accepted you can keep your changes at the beginning or at the end of platforms.json which would make a diff and manually merging in the repo changes into your local changes straightforward.

HTH

@s1eve-mcdichae1
Copy link
Author

The changes in the PR will put any new files from this repo with the suffix .rp-dist (this suffix is the RetroPie way) alongside to the file with any user made. Thus you will end up with a platforms.json and platforms.json.rp-dist.

Yeah that looks like probably the best solution for us at this time. This is what I meant about not receiving updates automatically but, really, I suppose that's not like something that's going to happen frequently anyway.

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

No branches or pull requests

2 participants