Merge pull request #1426 from heinezen/lgtm

Converter code cleanup

doc/ide/vscode.md

@@ -67,13 +67,15 @@ If you use a C/C++ language support extension, you need to add the path to the *
 
 If you use a Python language support extension with `autopep8` formatting, you should add these settings to get the openage Python code style. Add these entries to the list of passed arguments:
 
-* `--ignore E221,E241,E251,E501`
+* `--ignore`
+* `E221,E241,E251,E501`
 
 Ignoring the `E221,E241,E251` errors allows placing multiple spaces around commas and before comments for alignment, while ignoring `E501` disables auto-formatting of overlong lines.
 
 If you don't want to ignore `E501`, you should add this argument which increases the maximum line length to 100:
 
-* `--line-length 100`
+* `--max-line-length`
+* `100`
 
 
 ## Add openage configure and build tasks

openage/convert/entity_object/conversion/converter_object.py

@@ -380,7 +380,7 @@ def extend_raw_member(self, name, push_value, origin):
             member_origin = raw_member[2]
 
             if name == member_name and member_origin == origin:
-                member_value = member_value.extend(push_value)
+                member_value.extend(push_value)
                 break
 
         else:

openage/convert/entity_object/conversion/stringresource.py

@@ -5,7 +5,6 @@
 from collections import defaultdict
 
 from ...entity_object.conversion.genie_structure import GenieStructure
-from ...entity_object.export import data_definition
 
 
 class StringResource(GenieStructure):
@@ -27,21 +26,6 @@ def get_tables(self):
         """
         return self.strings
 
-    def dump(self, filename):
-        data = []
-
-        for lang, langstrings in self.strings.items():
-            for idx, text in langstrings.items():
-                entry = {
-                    "id": idx,
-                    "lang": lang,
-                    "text": text,
-                }
-                data.append(entry)
-
-        data = sorted(data, key=lambda x: x["id"])
-        return [data_definition.DataDefinition(self, data, filename)]
-
     @classmethod
     def get_data_format_members(cls, game_version):
         """

openage/convert/processor/conversion/aoc/ability_subprocessor.py

@@ -4073,7 +4073,6 @@ def idle_ability(line):
 
                 if civ_animation_id != ability_animation_id:
                     # Find the corresponding graphics set
-                    graphics_set_id = -1
                     for set_id, items in gset_lookup_dict.items():
                         if civ_id in items[0]:
                             graphics_set_id = set_id
@@ -6739,7 +6738,6 @@ def trade_ability(line):
         # Trade route (use the trade route to the market)
         trade_routes = []
 
-        trade_post_id = -1
         unit_commands = current_unit["unit_commands"].get_value()
         for command in unit_commands:
             # Find the trade command and the trade post id

openage/convert/processor/conversion/aoc/modifier_subprocessor.py

@@ -1,9 +1,6 @@
 # Copyright 2020-2021 the openage authors. See copying.md for legal info.
 #
-# pylint: disable=too-many-locals,too-many-branches,too-many-nested-blocks
-#
-# TODO:
-# pylint: disable=line-too-long
+# pylint: disable=too-many-locals,too-many-branches,too-many-nested-blocks,too-many-statements
 
 """
 Derives and adds abilities to lines or civ groups. Subroutine of the
@@ -34,8 +31,12 @@ def elevation_attack_modifiers(converter_obj_group):
         """
         dataset = converter_obj_group.data
         modifiers = [
-            dataset.pregen_nyan_objects["util.modifier.elevation_difference.AttackHigh"].get_nyan_object(),
-            dataset.pregen_nyan_objects["util.modifier.elevation_difference.AttackLow"].get_nyan_object()
+            dataset.pregen_nyan_objects[
+                "util.modifier.elevation_difference.AttackHigh"
+            ].get_nyan_object(),
+            dataset.pregen_nyan_objects[
+                "util.modifier.elevation_difference.AttackLow"
+            ].get_nyan_object()
         ]
 
         return modifiers
@@ -51,7 +52,9 @@ def flyover_effect_modifier(converter_obj_group):
         :rtype: ...dataformat.forward_ref.ForwardRef
         """
         dataset = converter_obj_group.data
-        modifier = dataset.pregen_nyan_objects["util.modifier.flyover_cliff.AttackFlyover"].get_nyan_object()
+        modifier = dataset.pregen_nyan_objects[
+            "util.modifier.flyover_cliff.AttackFlyover"
+        ].get_nyan_object()
 
         return modifier
 
@@ -89,14 +92,16 @@ def gather_rate_modifier(converter_obj_group):
                     # Find a gather ability.
                     type_id = command["type"].get_value()
 
-                    if type_id not in (5, 110):
+                    gather_task_ids = internal_name_lookups.get_gather_lookups(
+                        dataset.game_version).keys()
+                    if type_id not in gather_task_ids:
                         continue
 
                     work_value = command["work_value1"].get_value()
 
                     # Check if the work value is 1 (with some rounding error margin)
-                    # if not we have to create a modifier
-                    if work_value < 1.0001 or work_value > 0.9999:
+                    # if not, we have to create a modifier
+                    if 0.9999 < work_value < 1.0001:
                         continue
 
                     # Search for the lines with the matching class/unit id
@@ -118,6 +123,9 @@ def gather_rate_modifier(converter_obj_group):
                             if line.get_class_id() == class_id:
                                 lines.append(line)
 
+                    else:
+                        raise Exception("Gather task has no valid target ID.")
+
                     # Create a modifier for each matching resource spot
                     for resource_line in lines:
                         head_unit_id = resource_line.get_head_unit_id()
@@ -135,22 +143,27 @@ def gather_rate_modifier(converter_obj_group):
                         modifier_raw_api_object = RawAPIObject(modifier_ref,
                                                                "%sGatheringRate",
                                                                dataset.nyan_api_objects)
-                        modifier_raw_api_object.add_raw_parent("engine.modifier.multiplier.type.GatheringRate")
+                        modifier_raw_api_object.add_raw_parent(
+                            "engine.modifier.multiplier.type.GatheringRate")
                         modifier_location = ForwardRef(converter_obj_group, target_obj_name)
                         modifier_raw_api_object.set_location(modifier_location)
 
                         # Multiplier
-                        modifier_raw_api_object.add_raw_member("multiplier",
-                                                               work_value,
-                                                               "engine.modifier.multiplier.MultiplierModifier")
+                        modifier_raw_api_object.add_raw_member(
+                            "multiplier",
+                            work_value,
+                            "engine.modifier.multiplier.MultiplierModifier"
+                        )
 
                         # Resource spot
                         spot_ref = "%s.Harvestable.%sResourceSpot" % (resource_line_name,
                                                                       resource_line_name)
                         spot_forward_ref = ForwardRef(resource_line, spot_ref)
-                        modifier_raw_api_object.add_raw_member("resource_spot",
-                                                               spot_forward_ref,
-                                                               "engine.modifier.multiplier.type.GatheringRate")
+                        modifier_raw_api_object.add_raw_member(
+                            "resource_spot",
+                            spot_forward_ref,
+                            "engine.modifier.multiplier.type.GatheringRate"
+                        )
 
                         converter_obj_group.add_raw_api_object(modifier_raw_api_object)
                         modifier_forward_ref = ForwardRef(converter_obj_group,

openage/convert/processor/conversion/aoc/processor.py

@@ -511,7 +511,6 @@ def create_unit_lines(full_data_set):
 
             # Search other_connections for the previous unit in line
             connected_types = connection["other_connections"].get_value()
-            connected_index = -1
             for index, _ in enumerate(connected_types):
                 connected_type = connected_types[index]["other_connection"].get_value()
                 if connected_type == 2:
@@ -645,7 +644,6 @@ def create_building_lines(full_data_set):
                     continue
 
                 # Find the previous building
-                connected_index = -1
                 for c_index, _ in enumerate(connected_types):
                     connected_type = connected_types[c_index]["other_connection"].get_value()
                     if connected_type == 1:

openage/convert/processor/conversion/swgbcc/ability_subprocessor.py

@@ -1719,7 +1719,6 @@ def trade_ability(line):
         # Trade route (use the trade route o the market)
         trade_routes = []
 
-        trade_post_id = -1
         unit_commands = current_unit["unit_commands"].get_value()
         for command in unit_commands:
             # Find the trade command and the trade post id

openage/convert/processor/conversion/swgbcc/processor.py

@@ -235,7 +235,6 @@ def create_unit_lines(full_data_set):
 
             # Search other_connections for the previous unit in line
             connected_types = connection["other_connections"].get_value()
-            connected_index = -1
             for index, _ in enumerate(connected_types):
                 connected_type = connected_types[index]["other_connection"].get_value()
                 if connected_type == 2:
@@ -438,7 +437,6 @@ def create_building_lines(full_data_set):
 
             # Search other_connections for the previous unit in line
             connected_types = connection["other_connections"].get_value()
-            connected_index = -1
             for index, _ in enumerate(connected_types):
                 connected_type = connected_types[index]["other_connection"].get_value()
                 if connected_type == 1:

openage/convert/processor/export/media_exporter.py

@@ -4,14 +4,15 @@
 """
 Converts media requested by export requests to files.
 """
+import logging
 import os
 
 from openage.convert.entity_object.export.texture import Texture
 from openage.convert.service import debug_info
 from openage.convert.service.export.load_media_cache import load_media_cache
 from openage.convert.value_object.read.media.blendomatic import Blendomatic
 from openage.convert.value_object.read.media_types import MediaType
-from openage.log import dbg
+from openage.log import dbg, get_loglevel
 
 
 class MediaExporter:
@@ -124,10 +125,12 @@ def _export_blend(export_request, sourcedir, exportdir, blend_mode_count=None):
                 f"{export_request.target_filename}{idx}.png"
             )
 
-            MediaExporter.log_fileinfo(
-                source_file,
-                exportdir[export_request.targetdir, f"{export_request.target_filename}{idx}.png"]
-            )
+            if get_loglevel() <= logging.DEBUG:
+                MediaExporter.log_fileinfo(
+                    source_file,
+                    exportdir[export_request.targetdir,
+                              f"{export_request.target_filename}{idx}.png"]
+                )
 
     @staticmethod
     def _export_graphics(export_request, sourcedir, exportdir, palettes,
@@ -207,10 +210,12 @@ def _export_graphics(export_request, sourcedir, exportdir, palettes,
         export_request.set_changed()
         export_request.notify_observers(metadata)
         export_request.clear_changed()
-        MediaExporter.log_fileinfo(
-            source_file,
-            exportdir[export_request.targetdir, export_request.target_filename]
-        )
+
+        if get_loglevel() <= logging.DEBUG:
+            MediaExporter.log_fileinfo(
+                source_file,
+                exportdir[export_request.targetdir, export_request.target_filename]
+            )
 
     @staticmethod
     def _export_interface(export_request, sourcedir, **kwargs):
@@ -268,10 +273,11 @@ def _export_sound(export_request, sourcedir, exportdir):
         with export_file.open_w() as outfile:
             outfile.write(soundata)
 
-        MediaExporter.log_fileinfo(
-            source_file,
-            exportdir[export_request.targetdir, export_request.target_filename]
-        )
+        if get_loglevel() <= logging.DEBUG:
+            MediaExporter.log_fileinfo(
+                source_file,
+                exportdir[export_request.targetdir, export_request.target_filename]
+            )
 
     @staticmethod
     def _export_terrain(export_request, sourcedir, exportdir, palettes,
@@ -337,10 +343,11 @@ def _export_terrain(export_request, sourcedir, exportdir, palettes,
             compression_level,
         )
 
-        MediaExporter.log_fileinfo(
-            source_file,
-            exportdir[export_request.targetdir, export_request.target_filename]
-        )
+        if get_loglevel() <= logging.DEBUG:
+            MediaExporter.log_fileinfo(
+                source_file,
+                exportdir[export_request.targetdir, export_request.target_filename]
+            )
 
     @staticmethod
     def _get_media_cache(export_request, sourcedir, palettes, compression_level):
@@ -466,18 +473,23 @@ def log_fileinfo(source_file, target_file):
         source_format = source_file.suffix[1:].upper()
         target_format = target_file.suffix[1:].upper()
 
-        with source_file.open('r') as src:
-            src.seek(0, os.SEEK_END)
-            source_size = src.tell()
+        source_path = source_file.resolve_native_path()
+        if source_path:
+            source_size = os.path.getsize(source_path)
+
+        else:
+            with source_file.open('r') as src:
+                src.seek(0, os.SEEK_END)
+                source_size = src.tell()
 
-        with target_file.open('r') as dest:
-            dest.seek(0, os.SEEK_END)
-            target_size = dest.tell()
+        target_path = target_file.resolve_native_path()
+        target_size = os.path.getsize(target_path)
 
         log = ("Converted: "
                f"{source_file.name} "
-               f" ({source_format}, {source_size}B) "
+               f"({source_format}, {source_size}B) "
                f"-> {target_file.name} "
-               f"({target_format}, {target_size}B)")
+               f"({target_format}, {target_size}B | "
+               f"{(target_size / source_size * 100) - 100:+.1f}%)")
 
         dbg(log)

openage/convert/value_object/read/media/blendomatic.py

@@ -14,7 +14,6 @@
 
 from .....log import dbg
 from ....entity_object.conversion.genie_structure import GenieStructure
-from ....entity_object.export.data_definition import DataDefinition
 
 
 class BlendingTile:
@@ -254,32 +253,9 @@ def get_textures(self):
         one atlas per blending mode is generated,
         each atlas contains all blending masks merged on one texture
         """
-
         from ....entity_object.export.texture import Texture
         return [Texture(b_mode) for b_mode in self.blending_modes]
 
-    def dump(self, filename):
-        """
-        Return a printable file.
-        """
-        data = [
-            {"blend_mode": idx}
-            for idx, _ in enumerate(self.blending_modes)
-        ]
-        return [DataDefinition(self, data, filename)]
-
-    def save(self, fslikeobj, path, compression_level):
-        """
-        Save the blending mask textures to disk.
-        """
-
-        for idx, texture in enumerate(self.get_textures()):
-            name = "mode%02d.png" % idx
-            dbg("saving blending mode %02d texture -> %s", idx, name)
-            texture.save(fslikeobj, path + '/' + name, compression_level)
-
-        dbg("blending masks successfully exported")
-
     @classmethod
     def get_data_format_members(cls, game_version):
         """