Closed adellaci closed 5 years ago
Instead of opening an issue you should rather open a pull-request against one of the cores
@Phatcat I do not know how to do a pull request, at least with out making a mess of things.
@Phatcat i'll give it a shot again, but i would still like feed back on this.
It's quite hard to provide feedback on diffs without diffs ^_~
Pull requests aren't hard: you need to fork target repo to your own account, do the changes, push to your fork and then press the magic green button to request a pull.
That being said, no special git CLI magic involved, just your usual git workflow, the PR and fork -related stuff is handled through graphical interfaces of GitHub.
Check it out: https://help.github.com/articles/about-pull-requests/
@adellaci Just FYI, the existing script already supports using up to 8 cores.
@Muehe I did try it without using MoveMapGen8Core.sh but it error with a wrong parameter fault.
I tried a pull-request but i believe i have failed again.
How did you start it? And what was your error message exactly?
This branch is 5 commits ahead, 2802 commits behind cmangos:master.
https://github.com/adellaci/mangos-wotlk/commit/2043679d601237420ac82cb35c7d6771c9e7c32b
i dont feel well about your fork being 2800+ commits behind origin/master.
you should create a development branch on your fork, push your changes there and keep your fork udpated with origin/master.
@AnonXS Is there a way for me to delete my old one, and the weekend i'll jump on Discord to if someone is available to assist me in the correct way to do it?
@Muehe When get home it will rerun it and post the error
There are several ways to fix this, and we can help you with it on Discord. That being said, the files have been moved in the meantime, so the easiest solution would probably be to re-fork and manually apply the changes again.
@AnonXS adellaci/mangos-wotlk@2043679 This one is very old one, on the original work i had done to reorganize the map processing distribution to increase the speed of map processing, and add up to 8 processors. I initially was going to add more processor, but my research / testing resulted that anything more then 8 processor had no impact on reducing the time to process. That one has been commit and is the current script being used (I believe) and should have been close.
It was during that contribution that I messed things up trying to do it with pull request. I can not remember who it was that created it on my behalf, because of the mess i made. I'm not even sure if i ever thanked the individual that did that for me. <--- what an ungrateful "B" i am..LOL.
All that aside, I do want to learn the feature/function of 'git'. I started last night looking up tutorials, but all i have found, assumed that you understand the process some what. I know zero. I can clone & pull, but that is all i know.
@Muehe
When i first started writing the script change this was the first error
./ExtractResources.sh ........ sh: 0: Can't open MoveMapGen.sh
When i looked. MoveMapGen did not have a '.sh' extension.
so i changed the following line
sh MoveMapGen.sh $NUM_CPU $LOG_FILE $DETAIL_LOG_FILE
to
sh MoveMapGen $NUM_CPU $LOG_FILE $DETAIL_LOG_FILE
then reran with this error
MoveMapGen: 9: MoveMapGen: Syntax error: ")" unexpected
It was at that point i changed it to;
sh ./MoveMapGen8Core.sh $NUM_CPU $LOG_FILE $DETAIL_LOG_FILE
This worked as intended, so that is were i ended up in the change of the extractor script.
@adellaci Try this article: https://reflectoring.io/github-fork-and-pull/
@adellaci I have taken the liberty to just recreate your script changes on an up-to-date wotlk fork: https://github.com/Muehe/mangos-wotlk/tree/extract
I also changed the script shebang from #!/bin/sh
to #!/bin/bash
, like it is in the classic/tbc repo (has nothing to do with your changes).
Once I have checked that the script still works, I will cherry pick your commit (used the author info from your old commits btw., hope that's OK) into the other cores and open PRs.
As for learning to work with git, I can recommend this website: https://learngitbranching.js.org/
It has an interactive shell on the website and guides you through some common actions when working with git. Hands down the best introduction I found. If you are looking for additional info, some more links may be found in our wiki.
@ulfgebhardt Thank you very much for the info. @Muehe Thank you also for the info, and the commit, I do not mind at all.
Great.
Hint:
"#!/bin/bash" -> "#!/bin/sh" "AD_REZ" -> "AD_RES" "VMAP_REZ" -> "VMAP_RES" "hi-rez extraction" -> "high resolution extraction" or "high-res extraction"
@anyone31 I included your suggestions into my PRs, except the change from bash
to sh
. Why did you suggest that? I intentionally changed it on WotLK, because Classic and TBC already used bash
.
Example of current output:
$ ./ExtractResources.sh
Welcome to helper script to extract required dataz for MaNGOS!
Should all dataz (dbc, maps, vmaps and mmaps be extracted? (y/n)
y
How many CPUs should be used for extracting mmaps? (1, 2, 4, 8)
4
MMap extraction can be started delayed
If you do _not_ want MMap Extraction to start delayed, just press return
Else enter number followed by s for seconds, m for minutes, h for hours
Example: "3h" - will start mmap extraction in 3 hours
MMap Extraction Delay (leave blank for direct extraction):
1h
Would you like the extraction of maps to be high-resolution? (y/n)
y
Would you like the extraction of vmaps to be high-resolution? (y/n)
y
Current Settings:
Extract DBCs/maps: 1, Extract vmaps: 1, Extract mmaps: 1, Processes for mmaps: 4
maps extraction will be high-resolution
vmaps extraction will be high-resolution
MMap Extraction will be started delayed by 1h
Press (Enter) to continue, or interrupt with (CTRL+C)
bash not always should be available at /bin/bash. Some platforms have it at /usr/local/bin/bash
@Muehe first thank you for handling the github stuff, and @RussianE39 and you for your inputs.
On the topic of bash vs sh, reminded me on the work i added to "InstallFullDB.sh". We change that from shebang sh to bash in order to get this section of the script to display correctly. I do not remember the exact reason why it impacted it. That said, i do not see any part of this script that would impact how it displays or functions.
if [ "$FORCE_WAIT" != "NO" ]
then
echo "ATTENTION: Your database $DATABASE will be reset to WoTLK-DB!"
echo "Please bring your repositories up-to-date!"
echo "Press CTRL+C to exit"
# show a mini progress bar
for i in {1..10}
do
echo -ne .
sleep 1
done
echo .
fi```
i posted results of all three trunk in the respective trunk. In the results of all three extractions are similar time wise. It was the same as when I did testing on my research changes to MoveMapGen.sh for 8 cores. Still again map 1 is the longest to process. In looking back at what my results were i did discover who had done the pull and commit for me.
So thanks to @cyberium & @Muehe for both of your help on both of those.
As soon as i get a chance i'm going to enroll at github lab to train on how to do this myself.
@anyone31 @RussianE39 Well, the script was actually using bash
syntax until this PR was merged recently. The shebang being different on Wotlk was probably why it was made in the first place.
I have made the change to sh
on the PRs and in addition checked the script with this web tool. They define different warnings, I included fixes for SC2006 (change `date`
to $(date)
, cause POSIX) and ignored SC2086 and SC2162, since the strings used do not include backslashes/spaces/newlines/etc.
@adellaci For understanding Github PRs, you have to understand git branches/remotes first, once you understand that PRs are trivial. I recommend https://learngitbranching.js.org/ again, it helped me a lot in that regard.
Merged. Thanks @adellaci and @anyone31 :+1:
🚀 Feature
High Resolution Extraction of maps and/or vmaps I modified the ExtractResources.sh to add the option to add hi rez maps & vmaps extraction, and to restore MoveMapGen8Core.sh to manage executing of MoveMapGen with the use of all available processors up to 8 cores.
I've tested it. I would like confirmation, and someone to submit it.
Code ExtractResources.sh
Code MoveMapGen8Core.sh