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

Use concurrent.futures and improve user feedback in mmap_extract.py. #2559

Merged

Conversation

mserajnik
Copy link
Contributor

@mserajnik mserajnik commented Mar 20, 2024

🍰 Pullrequest

This is a continuation of #2541 and improves the code of mmap_extract.py further. It uses concurrent.futures for the subprocesses instead of the previous thread-based parallelism, which is more efficient than the thread-keepalive check that existed beforehand and also allows us to easily check whether the function raised any exception (i.e. the MoveMapGen executable returned a non-zero exit code), which is hard when subclassing threading.Thread. It also determines the platform-specific configuration for running the MoveMapGen executable only once and then caches it, instead of running the same logic in every new thread.

The output of the MoveMapGen executable is now swallowed and instead the user is presented with a simple overview that makes it easier to follow and see which maps have finished processing and if there were any errors. This should be more appropriate in non-debug scenarios where all the user cares about is the end result instead of seeing the progress of each individual tile. It looks like this:

Using 8 worker(s) for map processing
(1/43) Map #34: Processed successfully
(2/43) Map #35: Processed successfully
(3/43) Map #13: Processed successfully
(4/43) Map #29: Processed successfully
(5/43) Map #42: Processed successfully
(6/43) Map #44: Processed successfully
(7/43) Map #43: Processed successfully
(8/43) Map #33: Processed successfully
(9/43) Map #70: Processed successfully
(10/43) Map #48: Processed successfully
(11/43) Map #36: Processed successfully
(12/43) Map #30: Processed successfully
(13/43) Map #37: Processed successfully
(14/43) Map #90: Processed successfully
(15/43) Map #129: Processed successfully
(16/43) Map #109: Processed successfully
(17/43) Map #189: Processed successfully
(18/43) Map #249: Processed successfully
(19/43) Map #229: Processed successfully
(20/43) Map #209: Processed successfully
(21/43) Map #230: Processed successfully
(22/43) Map #289: Processed successfully
(23/43) Map #47: Processed successfully
(24/43) Map #349: Processed successfully
(25/43) Map #269: Processed successfully
(26/43) Map #309: Processed successfully
(27/43) Map #389: Processed successfully
(28/43) Map #409: Processed successfully
(29/43) Map #449: Processed successfully
(30/43) Map #450: Processed successfully
(31/43) Map #369: Processed successfully
(32/43) Map #489: Processed successfully
(33/43) Map #169: Processed successfully
(34/43) Map #529: Processed successfully
(35/43) Map #451: Processed successfully
(36/43) Map #329: Processed successfully
(37/43) Map #429: Processed successfully
(38/43) Map #469: Processed successfully
(39/43) Map #509: Processed successfully
(40/43) Map #533: Processed successfully
(41/43) Map #531: Processed successfully
(42/43) Map #0: Processed successfully
(43/43) Map #1: Processed successfully
Finished processing all maps (43) with 0 error(s)

Finally, I've decided to look up the initial CMaNGOS commit that introduced the script and adjusted the copyright notice accordingly. If that's not desirable I can remove it, of course, but it would be a lot more accurate compared to the current situation (where VMaNGOS isn't even mentioned).

Proof

  • None

Issues

  • None

How2Test

  1. Put mmap_extract.py next to MoveMapGen
  2. Change the current working directory to the directory the data is in (= the directory Buildings, maps etc. is in)
  3. Run mmap_extract.py (from the data directory)

Todo / Checklist

  • None

@ratkosrb
Copy link
Contributor

you should put the current year in the vmangos line

@mserajnik
Copy link
Contributor Author

you should put the current year in the vmangos line

You mean like this?

Copyright (C) 2013-2017  CMaNGOS  https://github.com/cmangos
Copyright (C) 2017-2024  VMaNGOS  https://github.com/vmangos

@ratkosrb
Copy link
Contributor

No, the starting year. VMaNGOS didn't even exist in 2017.

@mserajnik
Copy link
Contributor Author

VMaNGOS didn't even exist in 2017.

Ah, sorry, I wasn't aware; I based the 2017 on c9151a5.

So like this?

Copyright (C) 2013-2017  CMaNGOS  https://github.com/cmangos
Copyright (C) 2024-present  VMaNGOS  https://github.com/vmangos

Or would you prefer this so there isn't any gap between the years?

Copyright (C) 2013-2024  CMaNGOS  https://github.com/cmangos
Copyright (C) 2024-present  VMaNGOS  https://github.com/vmangos

@ratkosrb
Copy link
Contributor

This is better. And that first commit was during Elysium. I created the vmangos repository to continue developing the software independent of any private server in late 2018.

@mserajnik mserajnik force-pushed the improve-mmap-extract-python-script branch from 21c9c48 to 7ee1481 Compare March 20, 2024 20:37
@mserajnik
Copy link
Contributor Author

Changed it to be like this now:

Copyright (C) 2013-2017  CMaNGOS  https://github.com/cmangos
Copyright (C) 2024-present  VMaNGOS  https://github.com/vmangos

Just let me know if you meant the second variant was better. 😄

@ratkosrb ratkosrb merged commit 1d0d1ed into vmangos:development Mar 28, 2024
3 checks passed
@mserajnik mserajnik deleted the improve-mmap-extract-python-script branch April 18, 2024 14:50
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

Successfully merging this pull request may close these issues.

2 participants