Skip to content

Commit eb64121

Browse files
committed
review fixes
1 parent 2226644 commit eb64121

3 files changed

Lines changed: 103 additions & 9 deletions

File tree

AGENTS.md

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,50 @@ This pipeline handles:
244244

245245
---
246246

247-
## 8. Backend Parity and Lua Guardrails
247+
## 8. Shared Project Config, Patch Targets, and Run Behavior
248+
249+
Recent Grill/compiler integration work moved `wurst.build` parsing rules into a tiny shared dependency. Keep compiler behavior aligned with that shared model.
250+
251+
### Shared project config dependency
252+
253+
* `de.peeeq.wurstscript/build.gradle` depends on `com.github.wurstscript:wurst-project-config`.
254+
* `de.peeeq.wurstio.languageserver.WurstBuildConfig` is a compiler adapter around the shared model, not a second config DAO.
255+
* Do not duplicate YAML parsing rules, patch aliases, or script-mode behavior in compiler-only code unless it is truly compiler-specific.
256+
* Preserve exact `wc3Patch` names for cache invalidation and diagnostics. Broad patch kind is useful for behavior choices, but not enough for hashes.
257+
258+
### Patch target rules
259+
260+
* Use the shared `Wc3PatchTarget` parser for `wc3Patch`.
261+
* Patch family boundaries:
262+
* below `1.29` => pre-1.29 behavior
263+
* `1.29` through `1.31` => classic
264+
* `1.32+`, `1.36`, `2.0`, and `Reforged-*` => Reforged
265+
* Friendly names and jass-history dump names should resolve through shared config. Do not add one-off aliases in compiler code.
266+
* If jass-history has a broken folder name, fix `wurstscript/jass-history` instead of compensating here.
267+
268+
### Build vs run
269+
270+
* Build/typecheck should prefer pinned `wc3Patch` from `wurst.build` and should not parse the installed Warcraft executable just to decide target patch data.
271+
* Config injection should use the pinned project patch when available, not the locally installed game patch.
272+
* User-facing executable version parsing failures must stay short. Do not print PE parser stack traces unless explicit debug logging is requested.
273+
* Run/launch is different from build: the selected Warcraft executable controls launch arguments and map placement.
274+
* When project patch family and selected client family differ, warn and allow the user to choose a different Warcraft III folder.
275+
* If launch folder selection changes the client, all launch decisions must use that selected `W3InstallationData`, not stale request-level `w3data`.
276+
* Legacy clients that need install-dir map placement must copy to the selected launch install's `Maps/Test` folder.
277+
278+
### Focused tests
279+
280+
For config and run-pipeline changes, prefer these focused checks before broader test runs:
281+
282+
```
283+
./gradlew test --tests tests.wurstscript.tests.WurstBuildConfigTests
284+
./gradlew test --tests tests.wurstscript.tests.MapRequestPatchTargetTests
285+
./gradlew make_for_userdir
286+
```
287+
288+
---
289+
290+
## 9. Backend Parity and Lua Guardrails
248291

249292
Recent fixes established additional rules for backend work. Follow these for all future changes:
250293

@@ -288,7 +331,7 @@ Recent fixes established additional rules for backend work. Follow these for all
288331

289332
---
290333

291-
## 9. Virtual Slot Binding and Determinism (New Generics + Lua)
334+
## 10. Virtual Slot Binding and Determinism (New Generics + Lua)
292335

293336
Recent regressions showed that virtual-slot binding can silently degrade to base/no-op implementations in generated Lua while still compiling. Follow these rules for all related changes:
294337

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/RunMap.java

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ private void startGame(WurstGui gui, CompilationResult result) throws Exception
145145
}
146146
Optional<GameVersion> detectedGameVersion = launchData.getWc3PatchVersion();
147147
if (buildConfig.shouldCopyRunMapToWarcraftMapDir(detectedGameVersion)) {
148-
mapCopy = copyToWarcraftMapDir(cachedMapFile.get());
148+
mapCopy = copyToWarcraftMapDir(cachedMapFile.get(), launchData);
149149
}
150150

151151

@@ -353,7 +353,7 @@ private String getWorkspaceAbsolute() {
353353
* <p>
354354
* This directory depends on warcraft version and whether we are on windows or wine is used.
355355
*/
356-
private File copyToWarcraftMapDir(File testMap) throws IOException {
356+
private File copyToWarcraftMapDir(File testMap, W3InstallationData launchData) throws IOException {
357357
String testMapName = "WurstTestMap.w3x";
358358
for (String arg : compileArgs) {
359359
if (arg.startsWith("-runmapTarget")) {
@@ -371,7 +371,7 @@ private File copyToWarcraftMapDir(File testMap) throws IOException {
371371
}
372372

373373
File myDocumentsFolder = FileSystemView.getFileSystemView().getDefaultDirectory();
374-
Optional<String> documentPath = findMapDocumentPath(testMapName, myDocumentsFolder);
374+
Optional<String> documentPath = findMapDocumentPath(testMapName, myDocumentsFolder, launchData);
375375

376376
// copy the map to the appropriate directory
377377
Optional<File> testFolder = documentPath.map(path -> new File(path, "Maps" + File.separator + "Test"));
@@ -404,7 +404,7 @@ private File copyToWarcraftMapDir(File testMap) throws IOException {
404404
return null;
405405
}
406406

407-
private Optional<String> findMapDocumentPath(String testMapName, File myDocumentsFolder) {
407+
private Optional<String> findMapDocumentPath(String testMapName, File myDocumentsFolder, W3InstallationData launchData) {
408408
Optional<String> documentPath = Optional.of(
409409
langServer.getConfigProvider().getMapDocumentPath().orElseGet(
410410
() -> myDocumentsFolder.getAbsolutePath() + File.separator + "Warcraft III"));
@@ -425,13 +425,13 @@ private Optional<String> findMapDocumentPath(String testMapName, File myDocument
425425
}
426426
}
427427

428-
if (buildConfig.shouldUseInstallDirForMaps(w3data.getWc3PatchVersion())) {
428+
if (buildConfig.shouldUseInstallDirForMaps(launchData.getWc3PatchVersion())) {
429429
// 1.27 and lower compat
430430
WLogger.info("Version 1.27 or lower detected, changing file location");
431-
documentPath = wc3Path;
431+
documentPath = mapInstallDirectoryForLegacyLaunch(launchData, wc3Path);
432432
} else {
433433
// For 1.28+ the wc3/maps/test folder must not contain a map of the same name
434-
Optional<File> oldFile = wc3Path.map(
434+
Optional<File> oldFile = installRootForLaunchData(launchData).map(File::getAbsolutePath).or(() -> wc3Path).map(
435435
w3p -> new File(w3p, "Maps" + File.separator + "Test" + File.separator + testMapName));
436436
if (oldFile.isPresent() && oldFile.get().exists()) {
437437
if (!oldFile.get().delete()) {
@@ -441,4 +441,32 @@ private Optional<String> findMapDocumentPath(String testMapName, File myDocument
441441
}
442442
return documentPath;
443443
}
444+
445+
private static Optional<String> mapInstallDirectoryForLegacyLaunch(W3InstallationData launchData, Optional<String> fallbackWc3Path) {
446+
Optional<File> launchRoot = installRootForLaunchData(launchData);
447+
if (launchRoot.isPresent()) {
448+
return launchRoot.map(File::getAbsolutePath);
449+
}
450+
return fallbackWc3Path;
451+
}
452+
453+
private static Optional<File> installRootForLaunchData(W3InstallationData launchData) {
454+
return launchData.getGameExe().map(RunMap::installRootForExecutable);
455+
}
456+
457+
private static File installRootForExecutable(File executable) {
458+
File parent = executable.getAbsoluteFile().getParentFile();
459+
if (parent == null) {
460+
return executable.getAbsoluteFile();
461+
}
462+
if (parent.getName().equalsIgnoreCase("x86") || parent.getName().equalsIgnoreCase("x86_64")) {
463+
File maybeRetail = parent.getParentFile();
464+
if (maybeRetail != null && (maybeRetail.getName().equalsIgnoreCase("_retail_") || maybeRetail.getName().equalsIgnoreCase("_ptr_"))) {
465+
File installRoot = maybeRetail.getParentFile();
466+
return installRoot != null ? installRoot : maybeRetail;
467+
}
468+
return maybeRetail != null ? maybeRetail : parent;
469+
}
470+
return parent;
471+
}
444472
}

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/MapRequestPatchTargetTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,29 @@ public void runMapInfersClassicClientFromWar3ExecutablePath() throws Exception {
8686
assertEquals(request.detectedVersion().orElseThrow(), new GameVersion("1.28"));
8787
}
8888

89+
@Test
90+
public void legacyRunMapPlacementUsesSelectedLaunchInstall() throws Exception {
91+
Path selectedInstall = Files.createTempDirectory("warcraft-selected-legacy");
92+
Path exe = Files.writeString(selectedInstall.resolve("war3.exe"), "not a PE file");
93+
Path staleFallback = Files.createTempDirectory("warcraft-stale-fallback");
94+
W3InstallationData launchData = new W3InstallationData(
95+
Optional.of(exe.toFile()),
96+
Optional.of(new GameVersion("1.27"))
97+
);
98+
99+
Method placementRoot = RunMap.class.getDeclaredMethod(
100+
"mapInstallDirectoryForLegacyLaunch",
101+
W3InstallationData.class,
102+
Optional.class
103+
);
104+
placementRoot.setAccessible(true);
105+
106+
@SuppressWarnings("unchecked")
107+
Optional<String> result = (Optional<String>) placementRoot.invoke(null, launchData, Optional.of(staleFallback.toString()));
108+
109+
assertEquals(result.orElseThrow(), selectedInstall.toFile().getAbsolutePath());
110+
}
111+
89112
@Test
90113
public void configuredVersionSurvivesManualPathLoad() throws Exception {
91114
W3InstallationData data = new W3InstallationData(Optional.empty(), Optional.of(new GameVersion("2.0")));

0 commit comments

Comments
 (0)