L0laapk3 / FactorioMaps

L0laapk3's FactorioMaps mod
https://mods.factorio.com/mod/L0laapk3_FactorioMaps
Other
121 stars 22 forks source link

Linux compatibility #10

Closed OvermindDL1 closed 5 years ago

OvermindDL1 commented 5 years ago

Had a number of issues related to the python scripts and the final generated webpage.

Python script issues

  1. It appears that the python script used is out of date (I am using Python 2.7.12), followed the instructions to pip install Pillow psutil and it seems the latest version of psutil does not contain a psutil.BELOW_NORMAL_PRIORITY_CLASS value, thus the calls to psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10) are failing. I first had to comment all those out as I didn't really care about fixing it to the current methods as I'm just trying to get it working.

  2. The path separators appear to have // used, which is invalid in Python programs and will only incidentally/accidentally work on some limited systems. Either a path separator of / should be used, which 'works' on all systems Python supports but is still not the recommended way, or using os.path.join(...) (which is indeed used in 'most' placed) is the proper way to join paths in all cases. 'Fixed' all those via / as there were a number of constants around...

  3. There really should be more information about what the subprocess queues work status is as they seemed to hang on quite a number of occasions, even waiting over an hour did not see them complete, I had to force the subprocess count to just 1 to get them to work, at which case they worked properly. Did not look into what was happening as forcing the multiprocess count to 1 fixed it, perhaps a race condition or so.

  4. Although there is a commandline option of --factorio=path/to/factorio it seems to be defaulting checks to a variety of locations, none of which would just be ../../bin/x64/factorio, which is the default executable location for the *.tar.gz download, thus forcing me to put in it's full path every time.

  5. It appears to try killing Factorio with a taskkill command, which is not a valid program command, it should instead send an appropriate signal to the process instead, which will work everywhere that factorio runs on.

  6. The Popen command has invalid argument types passed to it, it is not a shell style command, rather it needs to be passed a list where the first element is the program and each additional element is a command line argument (sans quotes as this is, again, not a shell expansion).

  7. The subprocess.call calls are being used with shell expansion, but shell expansion mode is not enabled via shell=True.

A Quick and Dirty patch file to fix the above issues is:

diff --git a/auto.py b/auto.py
index f2545f7..9b9540d 100644
--- a/auto.py
+++ b/auto.py
@@ -22,16 +22,16 @@ python = sys.executable
 args = sys.argv[1:]
 kwargs = {}
 args = filter(parseArg, args)
-foldername = args[0] if len(args) > 0 else os.path.splitext(os.path.basename(max([os.path.join("..\\..\\saves", basename) for basename in os.listdir("..\\..\\saves") if basename not in { "_autosave1.zip", "_autosave2.zip", "_autosave3.zip" }], key=os.path.getmtime)))[0]
+foldername = args[0] if len(args) > 0 else os.path.splitext(os.path.basename(max([os.path.join("../../saves", basename) for basename in os.listdir("../../saves") if basename not in { "_autosave1.zip", "_autosave2.zip", "_autosave3.zip" }], key=os.path.getmtime)))[0]
 savenames = args[1:] or [ foldername ]

 possiblePaths = [
-    "C:\\Program Files\\Factorio\\bin\\x64\\factorio.exe",
-    "D:\\Program Files\\Factorio\\bin\\x64\\factorio.exe",
-    "C:\\Games\\Factorio\\bin\\x64\\factorio.exe",
-    "D:\\Games\\Factorio\\bin\\x64\\factorio.exe",
-    "C:\\Program Files (x86)\\Steam\\steamapps\\common\\Factorio\\bin\\x64\\factorio.exe",
-    "D:\\Program Files (x86)\\Steam\\steamapps\\common\\Factorio\\bin\\x64\\factorio.exe"
+    "C:/Program Files/Factorio/bin/x64/factorio.exe",
+    "D:/Program Files/Factorio/bin/x64/factorio.exe",
+    "C:/Games/Factorio/bin/x64/factorio.exe",
+    "D:/Games/Factorio/bin/x64/factorio.exe",
+    "C:/Program Files (x86)/Steam/steamapps/common/Factorio/bin/x64/factorio.exe",
+    "D:/Program Files (x86)/Steam/steamapps/common/Factorio/bin/x64/factorio.exe"
 ]
 try:
     factorioPath = next(x for x in ([kwargs["factorio"]] if "factorio" in kwargs else possiblePaths) if os.path.isfile(x))
@@ -40,9 +40,9 @@ except StopIteration:

 print(factorioPath)

-psutil.Process(os.getpid()).nice(psutil.ABOVE_NORMAL_PRIORITY_CLASS or 5)
+#psutil.Process(os.getpid()).nice(psutil.ABOVE_NORMAL_PRIORITY_CLASS or 5)

-basepath = kwargs["basepath"] if "basepath" in kwargs else "..\\..\\script-output\\FactorioMaps"
+basepath = kwargs["basepath"] if "basepath" in kwargs else "../../script-output/FactorioMaps"
 workthread = None

@@ -100,7 +100,7 @@ if os.path.isfile("autorun.lua"):
 print("enabling FactorioMaps mod")
 def changeModlist(newState):
     done = False
-    with open("..\\mod-list.json", "r") as f:
+    with open("../mod-list.json", "r") as f:
         modlist = json.load(f)
     for mod in modlist["mods"]:
         if mod["name"] == "L0laapk3_FactorioMaps":
@@ -108,7 +108,7 @@ def changeModlist(newState):
             done = True
     if not done:
         modlist["mods"].append({"name": "L0laapk3_FactorioMaps", "enabled": newState})
-    with open("..\\mod-list.json", "w") as f:
+    with open("../mod-list.json", "w") as f:
         json.dump(modlist, f, indent=2)

 changeModlist(True)
@@ -150,7 +150,7 @@ try:

         print("starting factorio")
-        p = subprocess.Popen(factorioPath + ' --load-game "' + savename + '" --disable-audio --no-log-rotation')
+        p = subprocess.Popen([factorioPath, '--load-game', savename, '--disable-audio', '--no-log-rotation'])

         if not os.path.exists(datapath):
             while not os.path.exists(datapath):
@@ -169,14 +169,14 @@ try:

         for screenshot in latest:
             print("Cropping %s images" % screenshot)
-            if 0 != call('%s crop.py %s %s' % (python, screenshot, basepath)): raise RuntimeError("crop failed")
+            if 0 != call('%s crop.py %s %s' % (python, screenshot, basepath), shell=True): raise RuntimeError("crop failed")

             def refZoom():
                 print("Crossreferencing %s images" % screenshot)
-                if 0 != call('%s ref.py %s %s' % (python, screenshot, basepath)): raise RuntimeError("ref failed")
+                if 0 != call('%s ref.py %s %s' % (python, screenshot, basepath), shell=True): raise RuntimeError("ref failed")
                 print("downsampling %s images" % screenshot)
-                if 0 != call('%s zoom.py %s %s' % (python, screenshot, basepath)): raise RuntimeError("zoom failed")
+                if 0 != call('%s zoom.py %s %s' % (python, screenshot, basepath), shell=True): raise RuntimeError("zoom failed")
             if screenshot != latest[-1]:
                 refZoom()
             else:
@@ -184,7 +184,7 @@ try:
                 if p.poll() is None:
                     p.kill()
                 else:
-                    os.system("taskkill /im factorio.exe")
+                    os.system("killall factorio")
                 if savename == savenames[-1]:
                     refZoom()
                 else:
@@ -230,7 +230,7 @@ except KeyboardInterrupt:
     if p.poll() is None:
         p.kill()
     else:
-        os.system("taskkill /im factorio.exe")
+        os.system("killall factorio")
     raise

 finally:
diff --git a/crop.py b/crop.py
index 02f9997..505c91a 100644
--- a/crop.py
+++ b/crop.py
@@ -24,10 +24,10 @@ def work(line, imgsize, folder):

 if __name__ == '__main__':

-    psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10)
+    #psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10)

-    subname = "\\".join(sys.argv[2:5])
-    toppath = os.path.join((sys.argv[5] if len(sys.argv) > 5 else "..\\..\\script-output\\FactorioMaps"), sys.argv[1])
+    subname = "/".join(sys.argv[2:5])
+    toppath = os.path.join((sys.argv[5] if len(sys.argv) > 5 else "../../script-output/FactorioMaps"), sys.argv[1])

     basepath = os.path.join(toppath, "Images", subname)

diff --git a/ref.py b/ref.py
index ed597f0..b6af396 100644
--- a/ref.py
+++ b/ref.py
@@ -72,10 +72,10 @@ def getBase64(number, isNight): #coordinate to 18 bit value (3 char base64)

 if __name__ == '__main__':

-    psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10)
+    #psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10)

-    toppath = os.path.join((sys.argv[5] if len(sys.argv) > 5 else "..\\..\\script-output\\FactorioMaps"), sys.argv[1])
+    toppath = os.path.join((sys.argv[5] if len(sys.argv) > 5 else "../../script-output/FactorioMaps"), sys.argv[1])
     datapath = os.path.join(toppath, "mapInfo.json")
     maxthreads = mp.cpu_count()

diff --git a/zoom.py b/zoom.py
index 14fdefd..fe04f29 100644
--- a/zoom.py
+++ b/zoom.py
@@ -102,10 +102,10 @@ def thread(basepath, pathList, surfaceName, daytime, start, stop, last, allChunk

 if __name__ == '__main__':

-       psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10)
+       #psutil.Process(os.getpid()).nice(psutil.BELOW_NORMAL_PRIORITY_CLASS or -10)

-       toppath = os.path.join((sys.argv[5] if len(sys.argv) > 5 else "..\\..\\script-output\\FactorioMaps"), sys.argv[1])
+       toppath = os.path.join((sys.argv[5] if len(sys.argv) > 5 else "../../script-output/FactorioMaps"), sys.argv[1])
        datapath = os.path.join(toppath, "mapInfo.json")
        basepath = os.path.join(toppath, "Images")
        maxthreads = mp.cpu_count()
@@ -155,7 +155,7 @@ if __name__ == '__main__':
                                                                                        allChunks.append((pos[0]*(2**threadsplit) + i, pos[1]*(2**threadsplit) + j))
                                                                                        queue.put(queue.qsize())

-                                                               threads = min(len(allChunks), maxthreads)
+                                                               threads = 1 # min(len(allChunks), maxthreads)
                                                                processes = []

                                                                print("%s %s %s %s" % (pathList[0], str(surfaceName), daytime, pathList))
@@ -191,4 +191,4 @@ if __name__ == '__main__':
                                                                        processes.append(p)
                                                                for p in processes:
                                                                        p.join()
-
\ No newline at end of file
+

Webpage issues

  1. The index.html has a weird <script src="/cdn-cgi/apps/head/ThhfSzyp6GI_EE-HfsckZnD6ThE.js"> tag at the top, which is a usual cloudflare tag that won't work anywhere else and is causing errors in the generated page.

  2. Doesn't work via a file:// schema but works via http:///https://, appears to be because of some improper scripting via some third-party library.

Notes

Once worked around these issues it seems to work quite well, fantastic idea. :-)

OvermindDL1 commented 5 years ago

As a documentative note, the only values that psutil exposes are:

╰─➤  python -i
Python 2.7.12 (default, Nov 12 2018, 14:36:49) 
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import psutil
>>> dir(psutil)
['AF_LINK', 'AccessDenied', 'CONN_CLOSE', 'CONN_CLOSE_WAIT', 'CONN_CLOSING', 'CONN_ESTABLISHED', 'CONN_FIN_WAIT1', 'CONN_FIN_WAIT2', 'CONN_LAST_ACK', 'CONN_LISTEN', 'CONN_NONE', 'CONN_SYN_RECV', 'CONN_SYN_SENT', 'CONN_TIME_WAIT', 'Error', 'IOPRIO_CLASS_BE', 'IOPRIO_CLASS_IDLE', 'IOPRIO_CLASS_NONE', 'IOPRIO_CLASS_RT', 'NIC_DUPLEX_FULL', 'NIC_DUPLEX_HALF', 'NIC_DUPLEX_UNKNOWN', 'NoSuchProcess', 'PROCFS_PATH', 'Popen', 'Process', 'RLIMIT_AS', 'RLIMIT_CORE', 'RLIMIT_CPU', 'RLIMIT_DATA', 'RLIMIT_FSIZE', 'RLIMIT_LOCKS', 'RLIMIT_MEMLOCK', 'RLIMIT_MSGQUEUE', 'RLIMIT_NICE', 'RLIMIT_NOFILE', 'RLIMIT_NPROC', 'RLIMIT_RSS', 'RLIMIT_RTPRIO', 'RLIMIT_RTTIME', 'RLIMIT_SIGPENDING', 'RLIMIT_STACK', 'RLIM_INFINITY', 'STATUS_DEAD', 'STATUS_DISK_SLEEP', 'STATUS_IDLE', 'STATUS_LOCKED', 'STATUS_RUNNING', 'STATUS_SLEEPING', 'STATUS_STOPPED', 'STATUS_TRACING_STOP', 'STATUS_WAITING', 'STATUS_WAKING', 'STATUS_ZOMBIE', 'TimeoutExpired', 'ZombieProcess', '_OPENBSD', '_POSIX', '_PY3', '_TOTAL_PHYMEM', '_WINDOWS', '__all__', '__author__', '__builtins__', '__doc__', '__file__', '__name__', '__package__', '__path__', '__version__', '_assert_pid_not_reused', '_common', '_compat', '_last_cpu_times', '_last_cpu_times_2', '_last_per_cpu_times', '_last_per_cpu_times_2', '_pmap', '_pslinux', '_psplatform', '_psposix', '_psutil_linux', '_psutil_posix', '_timer', 'boot_time', 'callable', 'collections', 'cpu_count', 'cpu_percent', 'cpu_times', 'cpu_times_percent', 'disk_io_counters', 'disk_partitions', 'disk_usage', 'errno', 'functools', 'long', 'net_connections', 'net_if_addrs', 'net_if_stats', 'net_io_counters', 'os', 'pid_exists', 'pids', 'process_iter', 'pwd', 'signal', 'subprocess', 'swap_memory', 'sys', 'test', 'time', 'traceback', 'users', 'version_info', 'virtual_memory', 'wait_procs']
OvermindDL1 commented 5 years ago

Also as note, I have to run it via python auto.py --factorio=/mnt/storage0/Factorio/bin/x64/factorio because python auto.py --factorio=../../bin/x64/factorio doesn't work, if it absname'd the path first then that would fix that I'd imagine.

L0laapk3 commented 5 years ago

Oh boy.

I assume you're using linux, which is why you were getting a whole list of problems, nice that you figured out what it took to get it to work at all :p

The taskkill is only there as a fallback, as when factorio is started trough steam, steam spawns a separate process and didn't let me control factorio.

You raise some good points, I'll have a more thorough look at everything when I find some time. :) Thanks for the efforts!

OvermindDL1 commented 5 years ago

Technically not just linux, but also mac and on some various windows setups as well (like running an enforced standards python). :-)

L0laapk3 commented 5 years ago
  1. Can you expand a bit on where it hangs with the multithreading? The queues are thread safe and I never had it hang.

  2. Can you please explain what you meant with "Doesn't work via a file:// schema"? I've had no problem like that.

  3. Its likely that you are using some other cjpeg on your system to convert from bitmaps to compressed jpegs than the one included since I only included the windows binaries for mozjpeg, is that correct?

I've made some commits on the dev branch, that should include fixes for hopefully all your other issues without creating new ones. But beware, I've changed the environment on the dev branch to python 3.7.1.

L0laapk3 commented 5 years ago

For 3: https://github.com/L0laapk3/FactorioMaps/blob/d65ad18b44c1146aab4c2fdc72fd59f0a3d5cc3a/zoom.py#L20-L28

OvermindDL1 commented 5 years ago

Can you expand a bit on where it hangs with the multithreading? The queues are thread safe and I never had it hang.

It hangs on the join calls, didn't look into the other multiprocess states to see where they themselves were frozen but they didn't join after almost 2 hours of waiting, where when I set the count to 1 it processed and join in <3 seconds.

Can you please explain what you meant with "Doesn't work via a file:// schema"? I've had no problem like that.

I tried it last night and the images weren't loading, but tried it again just now and it worked, no clue what's up... >.>

Its likely that you are using some other cjpeg on your system to convert from bitmaps to compressed jpegs than the one included since I only included the windows binaries for mozjpeg, is that correct?

╰─➤  cjpeg -version
libjpeg-turbo version 1.4.2 (build 20180705)

Very recent version as it's updated with the system itself. This is a pretty common program on linux so I'd generally expect it to be available, but it would be useful to add it on the readme as a requirement, say something like libjpeg-progs is required for apt-based systems or otherwise that libjpeg's programs are required to run. Or can make it optional and can test if it exists by calling it via cjpeg -version to see if you get a successful return result or not (if not just write out the unoptimized file).

Also you really Really shouldn't include platform-specific executable binaries in your git repository, it will throw a lot of check failures in some cases in addition your repository size is now permanently bloated with non-differencible data.

For 3:

I wouldn't, cjpeg is super common as a pre-installed thing on linux system, just try running the command, if it works then good, if it fails then fall back to just img.save(...).

L0laapk3 commented 5 years ago

The cjpeg is actually mozjpeg based on turbojpeg but with better compression, and its cjpeg is supposed to be a drop in replacement for the commong cjpeg. I'm only using this to gain a better compression at the cost of more processing time, as this mod is more geared towards actually hosting these maps online. However currently im not 100% happy with the implementation as there is a lot of overhead (spawning one process for every image). However actually compiling a better version of mozjpeg myself is not an easy task so I think i'll just leave it like this for now, as img.save basically uses libjpeg-turbo already.

L0laapk3 commented 5 years ago

I hope everything is solved now, let me know if there are any other problems :)

L0laapk3 commented 5 years ago

Feel free to reply to this if you still have issues related to this.

OvermindDL1 commented 5 years ago

I'll try to remember to update to the official branch again, I'm still using my modified fork. I had to add some throttling in it as my CPU was overheating a bit on the image work... ^.^;

/me really needs to upgrade...

L0laapk3 commented 5 years ago

The most efficient way to reduce heat is by leaving the program untouched but selecting a different power plan :) Also to clean out the device if you haven't done that in a while, especially if its a laptop :D

OvermindDL1 commented 5 years ago

Heh, I need to clean out my heatsink is what the issue is, but it's a... larger one than most and compressed air just kind of compacts it in more in the tiny slots among it. Doesn't help that it's 11 years old either... ^.^;