-
Notifications
You must be signed in to change notification settings - Fork 29
Handle MPQ maps without HM3W headers #1258
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,7 +107,7 @@ public static MapRequest.CompilationResult apply(WurstProjectConfigData projectC | |
| w3I.write(result.w3i); | ||
|
|
||
| // Apply map header (this is cheap, so we always do it) | ||
| applyMapHeader(projectConfig, targetMap); | ||
| applyMapHeader(projectConfig, targetMap, w3I.getPlayers().size(), w3I.getMapName(), w3I.getFlags().toInt()); | ||
|
|
||
| // Update the manifest with new config hash (must open writable to insert) | ||
| try (MpqEditor mpq = MpqEditorFactory.getEditor(Optional.of(targetMap), false)) { | ||
|
|
@@ -356,20 +356,50 @@ private static void setVolatilePlayerConfig(WurstProjectBuildPlayer wplayer, W3I | |
| } | ||
| } | ||
|
|
||
| private static void applyMapHeader(WurstProjectConfigData projectConfig, File targetMap) throws IOException { | ||
| private static void applyMapHeader(WurstProjectConfigData projectConfig, File targetMap, | ||
| int existingPlayerCount, String existingMapName, | ||
| int existingMapFlags) throws IOException { | ||
| boolean shouldWrite = false; | ||
| MapHeader mapHeader = MapHeader.ofFile(targetMap); | ||
| if (!projectConfig.buildMapData().players().isEmpty()) { | ||
| mapHeader.setMaxPlayersCount(projectConfig.buildMapData().players().size()); | ||
| WurstProjectBuildMapData buildMapData = projectConfig.buildMapData(); | ||
| if (buildMapData.players().isEmpty() && StringUtils.isBlank(buildMapData.name())) { | ||
| return; | ||
| } | ||
|
|
||
| // A Warcraft III map may omit the optional 512-byte HM3W prefix and start | ||
| // directly with its MPQ archive. MapHeader.ofFile only reads the prefix, | ||
| // so use a new header in that case; writeToMapFile will insert it before | ||
| // the archive. | ||
| boolean hasNoMapHeader = startsWithMpqArchive(targetMap); | ||
| MapHeader mapHeader = hasNoMapHeader | ||
| ? new MapHeader() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a headerless map configures Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 659e909. Header synthesis now receives the existing W3I map name and preserves it when player configuration changes without an explicit map name; the regression test covers that case. |
||
| : MapHeader.ofFile(targetMap); | ||
|
Comment on lines
+373
to
+375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a headerless MPQ whose W3I has nonzero map flags, such as fixed/custom forces or minimap settings, Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 7c10423. Header synthesis now copies the existing W3I flags via getFlags().toInt(), with regression coverage asserting the flags survive insertion. |
||
| if (!buildMapData.players().isEmpty()) { | ||
| mapHeader.setMaxPlayersCount(buildMapData.players().size()); | ||
| shouldWrite = true; | ||
| } else if (hasNoMapHeader) { | ||
| mapHeader.setMaxPlayersCount(existingPlayerCount); | ||
| } | ||
| if (hasNoMapHeader && StringUtils.isBlank(buildMapData.name())) { | ||
| mapHeader.setMapName(existingMapName); | ||
| } | ||
| if (StringUtils.isNotBlank(projectConfig.buildMapData().name())) { | ||
| mapHeader.setMapName(projectConfig.buildMapData().name()); | ||
| if (hasNoMapHeader) { | ||
| mapHeader.setFlags(existingMapFlags); | ||
| } | ||
| if (StringUtils.isNotBlank(buildMapData.name())) { | ||
| mapHeader.setMapName(buildMapData.name()); | ||
| shouldWrite = true; | ||
| } | ||
| if (shouldWrite) { | ||
| WLogger.info("Applying map header"); | ||
| mapHeader.writeToMapFile(targetMap); | ||
| } | ||
| } | ||
|
|
||
| private static boolean startsWithMpqArchive(File targetMap) throws IOException { | ||
| try (InputStream input = new FileInputStream(targetMap)) { | ||
| byte[] startToken = input.readNBytes(4); | ||
| return startToken.length == 4 | ||
| && new String(startToken, StandardCharsets.US_ASCII).startsWith("MPQ"); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a headerless map configures only
buildMapData.name()and relies on the existingwar3map.w3iplayer definitions, this constructs a header whose maximum-player count remains theMapHeaderdefault of zero (the new test explicitly confirms that value). The inserted HM3W header consequently advertises a zero-player map instead of preserving its existing capacity; populate the synthesized header from the already-extracted W3I metadata before writing it.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d54d514. Header synthesis now receives the existing W3I player count and preserves it when only map name configuration changes; the regression test asserts maxPlayersCount=3.