Added directory for migration scripts and added item_ball migration script #3997
Conversation
Added Jasper's migration script to migration directory
pkmnsnfrn
changed the title
|
I think the folder name should make it more clear that it contains scripts. Migration can mean a lot of things. /Edit: Maybe migration_scripts? |
|
Now that I think about it, I would remove the |
|
The main goal of these scripts is to be run once, then never again, so in that sense they are migration scripts. See also Edu's answer to #3952 |
Not everything though, right? Where would the python script go from PR #3856? It will run with every make assuming we agree on python support. Which I hope we will. didn't seem like there was much disagreement there and even SBird warmed up as long as outside libraries aren't introduced. Edu also suggested the name |
I was expecting it'd go in tools because it's, well, a tool, as opposed to this script, which is run once manually to migrate. If we ever implement triple layer tiles in expansion, SBird's script for that migration would also go in that folder |
I see. Yeah, that makes sense. Then |
| incs_to_check = glob.glob('./data/scripts/*.inc') # all .incs in the script folder | ||
| incs_to_check += glob.glob('./data/maps/*/scripts.inc') # all map scripts | ||
| pories_to_check = glob.glob('./data/scripts/*.pory') ## all .porys in the script folder | ||
| pories_to_check += glob.glob('./data/maps/*/scripts.pory') # all map scripts |
There was a problem hiding this comment.
Those need changing if you run it from a sub-folder right?
There was a problem hiding this comment.
Nah, I just tested it in a different Emerald and it seems to work, but somebody else should check on me on this.
There was a problem hiding this comment.
This is odd. Afaik . is the root folder so it shouldn't work? I'll test it tomorrow.
There was a problem hiding this comment.
Yep, just tested. It doesn't work. It needs to be changed to the parent directory.
Well, Idk. I guess a decision should be made here since you can run the script from the root folder python sub-folder/script.py. I guess it doesn't really matter but I would probably move into the scripts folder to run it if I didn't know better.
There was a problem hiding this comment.
Migration scripts are executed by running the following commands from the root directory of a developer's project.
chmod +x migration_scripts/*.py ;
python3 migration_scripts/*.py ;these are the instructions in the README. Do these instructions fail?
OR did you do:
cd migration_scripts ;
chmod +x *.py ;
python3 *.pyThere was a problem hiding this comment.
I understand that python3 migration_scripts/*.py works. I'm just saying that people will go into the sub-folder and run the script from there without reading the instructions. Which will not work.
Btw is there a reason to add the executable permissions? It works fine without.
There was a problem hiding this comment.
Ig it's not really an important conversation. Whether you go into the folder and run the script from there or specify the correct path to it does accomplish the same thing in the end. Maybe there is a way to sneak in an Error Message if you run it from the wrong path?
There was a problem hiding this comment.
Ah okay.
I don't think there's a good way to make sure it runs no matter where you run the script from. I could be wrong.
If a dev doesn't read the file called README, and then just runs a python script without context...
I'll remove the chmod line.
An error message would is a good idea!
Updated script per feedback rh-hideout#3997 (comment) Co-authored-by: Bassoonian <[email protected]>
Issue(s) that this PR fixes
Fixes #3996, fixes #3952
Discord contact info
I am
pkmnsnfrn. I have already started a discussion thread.