Skip to content

Commit

Permalink
Fix for tmpdir when loading multiple mission files
Browse files Browse the repository at this point in the history
This change makes sure that each mission file, when loaded, it extracts
resources to unique temp dir rather than a shared one, which would lead
to resource overwriting when working with multiple loaded missions. I
worked with two missions, and each had "briefing.png" attached. When I
saved them I noticed that one was overwriting the other.

I guess accessing map_resource.files["DEFAULT"].values() directly might
not be desired and if you want, I can remove it. I think if m.tmpdir is
good, then a validation that m1.tmpdir != m2.tmpdir should be enough.

NOTE that get_file_path works only when .miz is loaded. It does not work
properly (even before this patch) for missions generated by pydcs, where
resources are added, e.g:

m = dcs.Mission(terrain=dcs.terrain.Caucasus()) res_key =
m.add_picture_blue("tests/images/blue") calling
m.map_resource.get_file_path(res_key) would returns
"'cs/dcs/tests/images/blue.png'". But that's a separate problem to
solve.

With this patch the get_file_path() throws an exception, which is IMHO
better than silently working wrong.

tmpdir is optional since I didn't see the need of creating a new tmp
folder every time a new Mission() object is created.
  • Loading branch information
332fg-raven committed Jan 26, 2024
1 parent a9fde5c commit 2bcc42c
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 4 deletions.
12 changes: 8 additions & 4 deletions dcs/mission.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ def __init__(self, terrain: Optional[Terrain] = None) -> None:
self.current_group_id = 0
self.current_dict_id = 0
self.filename: Optional[str] = None
self.tmpdir: Optional[str] = None

self.translation = Translation(self)
self.map_resource = MapResource(self)
Expand Down Expand Up @@ -235,6 +236,7 @@ def load_file(self, filename: str, bypass_triggers: bool = False) -> List[Status
self.current_unit_id = 0
self.current_group_id = 0
self.current_dict_id = 0
self.tmpdir = tempfile.mkdtemp()
status = []

def loaddict(fname: str, mizfile: zipfile.ZipFile, reserved_files: List[str]) -> Dict[str, Any]:
Expand Down Expand Up @@ -2128,14 +2130,13 @@ def __init__(self, mission: Mission):

def load_from_dict(self, _dict, zipf: zipfile.ZipFile, lang='DEFAULT'):
_dict = _dict["mapResource"]

for key in _dict:
filename = _dict[key]
filepath = 'l10n/{lang}/{fn}'.format(lang=lang, fn=filename)
self.added_paths.append(filepath)

try:
extractedpath = zipf.extract(filepath, tempfile.gettempdir())
extractedpath = zipf.extract(filepath, self.mission.tmpdir)
self.add_resource_file(extractedpath, lang, key)
except KeyError as ke:
print(ke, file=sys.stderr)
Expand All @@ -2146,7 +2147,7 @@ def load_binary_files(self, zipf: zipfile.ZipFile, reserved_files: List[str]):
continue

try:
extractedpath = zipf.extract(filepath, tempfile.gettempdir())
extractedpath = zipf.extract(filepath, self.mission.tmpdir)
self.binary_files.append({
"path": os.path.abspath(extractedpath),
"respath": filepath,
Expand Down Expand Up @@ -2188,7 +2189,10 @@ def get_file_path(self, resource_key: ResourceKey, lang: str = 'DEFAULT'):
:param lang:
:return:
"""
return self.files[lang][resource_key.key][len(tempfile.gettempdir()) + len('/l10n//') + len(lang):]
if self.mission.tmpdir is None:
raise RuntimeError("get_file_path() only works for loaded missions.")

return self.files[lang][resource_key.key][len(self.mission.tmpdir) + len('/l10n//') + len(lang):]

def store(self, zipf: zipfile.ZipFile, lang='DEFAULT'):
d = {}
Expand Down
Binary file added tests/images/m1/briefing.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added tests/images/m2/briefing.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
29 changes: 29 additions & 0 deletions tests/test_mission.py
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,35 @@ def test_action_a_out_picture_u(self) -> None:

self.assertEqual(m_action, m2.triggerrules.triggers[0].actions[5])

def test_resource_name_conflict_in_two_missions(self) -> None:
m1_filename = "missions/saved.m1.miz"
m1 = dcs.Mission(terrain=dcs.terrain.Caucasus())
m1_img_path = 'tests/images/m1/briefing.png'
m1.add_picture_blue(m1_img_path)
m1.save(m1_filename)

m2_filename = "missions/saved.m2.miz"
m2 = dcs.Mission(terrain=dcs.terrain.Caucasus())
m2_img_path = 'tests/images/m2/briefing.png'
m2.add_picture_blue(m2_img_path)
m2.save(m2_filename)

# verify that when a mission file is loaded
# it creates a unique temp folders for it's
# resources. If that's not true m2, which is loaded
# last, would overwrite resources from m1, and if m1
# is safed after m2 is loaded, it's resources
# would not have the right content.

m1 = dcs.Mission()
m1.load_file(m1_filename)
m2 = dcs.Mission()
m2.load_file(m2_filename)

self.assertNotEqual(m1.tmpdir, m2.tmpdir)
self.assertNotEqual(list(m1.map_resource.files["DEFAULT"].values()),
list(m2.map_resource.files["DEFAULT"].values()))

def test_empty_mission_with_coalitions(self) -> None:
m = dcs.mission.Mission()
m_filename = "tests/missions/countries-without-units-on-the-map.miz"
Expand Down

0 comments on commit 2bcc42c

Please sign in to comment.