Skip to content

Commit 332bfbd

Browse files
FrottyFrotty
authored
Fix grill build command (#1169)
1 parent de7afc3 commit 332bfbd

File tree

5 files changed

+289
-77
lines changed

5 files changed

+289
-77
lines changed

‎AGENTS.md‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,139 @@ run:
187187
* New behavior must be documented through tests.
188188

189189

190+
191+
---
192+
193+
## 7. LSP Structure and Build Pipelines
194+
195+
This repository has multiple entry points that may trigger compilation/build behavior:
196+
197+
* **Language Server runtime**
198+
`de.peeeq.wurstio.languageserver.*`
199+
* **LSP build request**
200+
`de.peeeq.wurstio.languageserver.requests.BuildMap`
201+
* **CLI compiler entry point**
202+
`de.peeeq.wurstio.Main`
203+
* **CLI map build request**
204+
`de.peeeq.wurstio.languageserver.requests.CliBuildMap`
205+
206+
### LSP architecture (high-level)
207+
208+
* `WurstLanguageServer` wires LSP protocol handlers.
209+
* `LanguageWorker` serializes requests and file-change reconciliation.
210+
* `ModelManagerImpl` owns project model state (wurst files, dependencies, diagnostics).
211+
* User actions like build/start/tests are implemented in `languageserver.requests.*`.
212+
213+
### Build-map pipeline (centralized)
214+
215+
Map build behavior is centralized in:
216+
217+
* `MapRequest.executeBuildMapPipeline(...)`
218+
219+
Both:
220+
221+
* `BuildMap` (VSCode/LSP build command), and
222+
* `CliBuildMap` (CLI `-build`, used by grill)
223+
224+
must use that shared backend flow.
225+
226+
This pipeline handles:
227+
228+
1. map/cached-map preparation
229+
2. script extraction/config application
230+
3. compilation (Jass/Lua)
231+
4. script + map data injection (including imports/w3i)
232+
5. final output map write + MPQ compression finalization
233+
234+
### Lock handling policy
235+
236+
* `BuildMap` (LSP/UI) may use interactive retry/rename behavior for locked output files.
237+
* `CliBuildMap` must fail fast with a clear error for locked files (non-interactive environments).
238+
239+
### Agent guardrails for future changes
240+
241+
* Do **not** reintroduce separate build-map logic in `Main` or other call sites.
242+
* If map build behavior changes, update the shared `MapRequest` pipeline first, then keep wrappers thin.
243+
* Ensure CLI and LSP builds remain behaviorally aligned unless a difference is explicitly required and tested.
244+
245+
---
246+
247+
## 8. Backend Parity and Lua Guardrails
248+
249+
Recent fixes established additional rules for backend work. Follow these for all future changes:
250+
251+
### Jass/Lua feature parity
252+
253+
* New language/compiler features must be validated for **both Jass and Lua** backends.
254+
* Behavior should be as close as possible across backends.
255+
* If behavior differs, treat it as intentional only when:
256+
* the reason is backend/runtime-specific, and
257+
* the difference is documented in tests.
258+
259+
### Error behavior parity expectations
260+
261+
* Prefer matching Jass behavior semantically in Lua output.
262+
* Be explicit that Lua is stricter in some runtime cases where Jass may silently default/swallow invalid operations.
263+
* Do not rely on Lua strictness as a substitute for correct lowering/translation.
264+
265+
### Lua inliner safety: callback/function-reference boundaries
266+
267+
* On Lua target, do **not** inline across callback/function-reference-heavy sites (IM `ImFuncRef`-containing callees).
268+
* This avoids breaking callback context semantics (e.g. wrapper/xpcall/callback-native interactions such as force/group enum callbacks).
269+
* This is a structural rule, not a name-based exclusion.
270+
271+
### Lua locals limit fallback (>200 locals)
272+
273+
* Lua has a hard local-variable limit per function.
274+
* When a function exceeds the safe local threshold, rewrite locals to a locals-table fallback.
275+
* Requirements for fallback correctness:
276+
* locals-table declaration must be at function top before first use,
277+
* rewritten accesses must target the declared table (no global fallback),
278+
* nested block local initializations must be preserved,
279+
* use deterministic **numeric slot indices** (`tbl[1]`, `tbl[2]`, ...) rather than string keys.
280+
281+
### Regression testing requirements
282+
283+
* Any backend parity fix must add/adjust regression tests in `tests.wurstscript.tests.*`.
284+
* Include tests that check:
285+
* generated backend output shape for the affected backend,
286+
* no behavioral regression in the other backend when relevant,
287+
* known fragile cases (dispatch binding, inlining boundaries, locals spilling).
288+
289+
---
290+
291+
## 9. Virtual Slot Binding and Determinism (New Generics + Lua)
292+
293+
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:
294+
295+
### Root-slot correctness is mandatory
296+
297+
* For FSM-style dispatch (`currentState.<rootSlot>(...)`), each concrete subclass must bind that **same root slot** to its own most-specific implementation.
298+
* Never accept mappings where a subclass has its own update method but the dispatched root slot still points to `NoOpState_*` (or another base implementation).
299+
* When verifying generated Lua, always inspect both:
300+
* the slot invoked at call-site (`FSM_*update`), and
301+
* class table assignments for each sibling state class.
302+
303+
### Override-chain integrity (wrapper/bridge cases)
304+
305+
* If override wrappers/bridges are created, preserve transitive override links (`wrapper -> real override`) so deeper subclasses remain reachable during slot/name normalization.
306+
* Avoid transformations that disconnect root methods from concrete overrides in the method union graph.
307+
308+
### Deterministic Lua emission requirements
309+
310+
* Lua output must be deterministic for identical input (same input -> byte-identical output in test harness).
311+
* Any iteration over methods/supertypes/union groups used for naming or table assignment must be deterministic (stable ordering).
312+
* If multiple candidate methods exist for the same slot in a class, selection must be deterministic and must prefer the most specific non-abstract implementation for that class.
313+
314+
### Required regression tests for slot fixes
315+
316+
* Add a repro with:
317+
* `State<T:>`, `NoOpState<T:>`, `FSM<T:>`,
318+
* multiple sibling `NoOpState<Owner>` subclasses (including at least 4+ siblings),
319+
* early constant state instantiation,
320+
* root-slot call through `State<T>`.
321+
* In generated Lua assertions:
322+
* extract the actual dispatched slot name from `FSM_*update` call-site,
323+
* assert each concrete sibling class binds that slot to its own implementation,
324+
* assert no sibling binds that dispatched slot to `NoOpState_*`.
325+
* Add a compile-twice determinism assertion for the same repro input.

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/Main.java‎

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@
88
import de.peeeq.wurstio.gui.AboutDialog;
99
import de.peeeq.wurstio.gui.WurstGuiImpl;
1010
import de.peeeq.wurstio.languageserver.LanguageServerStarter;
11-
import de.peeeq.wurstio.languageserver.ProjectConfigBuilder;
1211
import de.peeeq.wurstio.languageserver.WFile;
12+
import de.peeeq.wurstio.languageserver.requests.CliBuildMap;
1313
import de.peeeq.wurstio.map.importer.ImportFile;
1414
import de.peeeq.wurstio.mpq.MpqEditor;
1515
import de.peeeq.wurstio.mpq.MpqEditorFactory;
16-
import de.peeeq.wurstio.utils.W3InstallationData;
1716
import de.peeeq.wurstscript.CompileTimeInfo;
1817
import de.peeeq.wurstscript.ErrorReporting;
1918
import de.peeeq.wurstscript.RunArgs;
@@ -31,8 +30,7 @@
3130
import java.lang.management.RuntimeMXBean;
3231
import java.nio.file.Path;
3332
import java.nio.file.Paths;
34-
import java.nio.file.StandardCopyOption;
35-
import java.util.LinkedList;
33+
import java.util.ArrayList;
3634
import java.util.List;
3735
import java.util.Optional;
3836

@@ -110,35 +108,42 @@ public static void main(String[] args) {
110108
}
111109

112110
try {
113-
WurstProjectConfigData projectConfig = null;
114-
Path buildDir = null;
115-
Optional<Path> target = Optional.empty();
116111
String workspaceroot = runArgs.getWorkspaceroot();
112+
RunArgs compileArgs = runArgs;
113+
List<String> mergedArgs = new ArrayList<>(asList(args));
114+
if (workspaceroot != null) {
115+
WLogger.info("workspaceroot: " + workspaceroot);
116+
List<String> argsList = getCompileArgs(WFile.create(workspaceroot));
117+
WLogger.info("workspaceroot: " + (argsList == null));
118+
mergedArgs.addAll(argsList);
119+
compileArgs = new RunArgs(mergedArgs);
120+
}
121+
117122
if (runArgs.isBuild() && runArgs.getInputmap() != null && workspaceroot != null) {
118123
Path root = Paths.get(workspaceroot);
119124
Path inputMap = root.resolve(runArgs.getInputmap());
120-
projectConfig = WurstProjectConfig.INSTANCE.loadProject(root.resolve(FILE_NAME));
121-
125+
WurstProjectConfigData projectConfig = WurstProjectConfig.INSTANCE.loadProject(root.resolve(FILE_NAME));
122126
if (java.nio.file.Files.exists(inputMap) && projectConfig != null) {
123-
buildDir = root.resolve("_build");
124-
java.nio.file.Files.createDirectories(buildDir);
125-
target = Optional.of(buildDir.resolve(projectConfig.getBuildMapData().getFileName() + ".w3x"));
126-
java.nio.file.Files.copy(inputMap, target.get(), StandardCopyOption.REPLACE_EXISTING);
127-
runArgs.setMapFile(target.get().toAbsolutePath().toString());
127+
CliBuildMap cliBuildMap = new CliBuildMap(
128+
WFile.create(root.toFile()),
129+
Optional.of(inputMap.toFile()),
130+
mergedArgs,
131+
Optional.empty(),
132+
gui
133+
);
134+
de.peeeq.wurstio.languageserver.ModelManager modelManager =
135+
new de.peeeq.wurstio.languageserver.ModelManagerImpl(root.toFile(), new de.peeeq.wurstio.languageserver.BufferManager());
136+
modelManager.buildProject();
137+
Object result = cliBuildMap.execute(modelManager);
138+
WLogger.info("map build success");
139+
System.out.println("Build succeeded. Output file: <" + result + ">");
140+
gui.sendProgress("Finished!");
141+
return;
128142
}
129143
}
130144

131145
String mapFilePath = runArgs.getMapFile();
132146

133-
RunArgs compileArgs = runArgs;
134-
if (workspaceroot != null) {
135-
WLogger.info("workspaceroot: " + workspaceroot);
136-
List<String> argList = new LinkedList<>(asList(args));
137-
List<String> argsList = getCompileArgs(WFile.create(workspaceroot));
138-
WLogger.info("workspaceroot: " + (argsList == null));
139-
argList.addAll(argsList);
140-
compileArgs = new RunArgs(argList);
141-
}
142147
CompilationProcess compilationProcess = new CompilationProcess(gui, compileArgs);
143148
@Nullable CharSequence compiledScript;
144149

@@ -161,14 +166,6 @@ public static void main(String[] args) {
161166
if (compiledScript != null) {
162167
File scriptFile = new File("compiled.j.txt");
163168
Files.write(compiledScript.toString().getBytes(Charsets.UTF_8), scriptFile);
164-
165-
if (projectConfig != null && target.isPresent()) {
166-
ProjectConfigBuilder.apply(projectConfig, target.get().toFile(), scriptFile, buildDir.toFile(),
167-
runArgs, new W3InstallationData(null, Paths.get(workspaceroot).toFile(), false));
168-
169-
WLogger.info("map build success");
170-
System.out.println("Build succeeded. Output file: <" + target.get().toAbsolutePath() + ">");
171-
}
172169
}
173170

174171
gui.sendProgress("Finished!");

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/BuildMap.java‎

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,13 @@
66
import de.peeeq.wurstio.languageserver.ModelManager;
77
import de.peeeq.wurstio.languageserver.WFile;
88
import de.peeeq.wurstio.languageserver.WurstLanguageServer;
9-
import de.peeeq.wurstio.mpq.MpqEditor;
10-
import de.peeeq.wurstio.mpq.MpqEditorFactory;
119
import de.peeeq.wurstscript.WLogger;
1210
import de.peeeq.wurstscript.attributes.CompileError;
1311
import de.peeeq.wurstscript.gui.WurstGui;
1412
import org.eclipse.lsp4j.MessageType;
1513

1614
import java.io.File;
1715
import java.io.IOException;
18-
import java.nio.file.Files;
1916
import java.util.List;
2017
import java.util.Optional;
2118

@@ -46,49 +43,7 @@ public Object execute(ModelManager modelManager) throws IOException {
4643
WLogger.info("buildMap " + map + " " + compileArgs);
4744
WurstGui gui = new WurstGuiImpl(workspaceRoot.getFile().getAbsolutePath());
4845
try {
49-
if (!map.isPresent()) {
50-
throw new RequestFailedException(MessageType.Error, "Map is null");
51-
}
52-
if (!map.get().exists()) {
53-
throw new RequestFailedException(MessageType.Error, map.get().getAbsolutePath() + " does not exist.");
54-
}
55-
56-
MapRequest.mapLastModified = map.get().lastModified();
57-
MapRequest.mapPath = map.get().getAbsolutePath();
58-
59-
gui.sendProgress("Copying map");
60-
61-
// first we copy in same location to ensure validity
62-
File buildDir = getBuildDir();
63-
String fileName = projectConfig.getBuildMapData().getFileName();
64-
File targetMapFile = new File(buildDir, fileName.isEmpty() ? projectConfig.getProjectName() + ".w3x" : fileName + ".w3x");
65-
targetMapFile = ensureWritableTargetFile(
66-
targetMapFile,
67-
"Build Map",
68-
"The output map file is in use and cannot be replaced.\nClose Warcraft III and click Retry, choose Rename to use a temporary file name, or Cancel.",
69-
"Build canceled because output map target is in use."
70-
);
71-
CompilationResult result = compileScript(modelManager, gui, Optional.of(targetMapFile), projectConfig, buildDir, true);
72-
73-
injectMapData(gui, Optional.of(targetMapFile), result);
74-
75-
targetMapFile = ensureWritableTargetFile(
76-
targetMapFile,
77-
"Build Map",
78-
"The output map file is still in use and cannot be replaced.\nClick Retry, choose Rename to use a temporary file name, or Cancel.",
79-
"Build canceled because output map target is in use."
80-
);
81-
Files.copy(getCachedMapFile().toPath(), targetMapFile.toPath(), java.nio.file.StandardCopyOption.REPLACE_EXISTING);
82-
83-
gui.sendProgress("Finalizing map");
84-
85-
try (MpqEditor mpq = MpqEditorFactory.getEditor(Optional.of(targetMapFile))) {
86-
if (mpq != null) {
87-
mpq.closeWithCompression();
88-
}
89-
}
90-
91-
gui.sendProgress("Done.");
46+
executeBuildMapPipeline(modelManager, gui, projectConfig);
9247
} catch (CompileError e) {
9348
WLogger.info(e);
9449
throw new RequestFailedException(MessageType.Error, "A compilation error occurred when building the map:\n" + e);

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/CliBuildMap.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package de.peeeq.wurstio.languageserver.requests;
2+
3+
import config.WurstProjectConfig;
4+
import config.WurstProjectConfigData;
5+
import de.peeeq.wurstio.languageserver.ModelManager;
6+
import de.peeeq.wurstio.languageserver.WFile;
7+
import de.peeeq.wurstscript.WLogger;
8+
import de.peeeq.wurstscript.attributes.CompileError;
9+
import de.peeeq.wurstscript.gui.WurstGui;
10+
import org.eclipse.lsp4j.MessageType;
11+
12+
import java.io.File;
13+
import java.io.IOException;
14+
import java.nio.channels.FileChannel;
15+
import java.nio.file.StandardOpenOption;
16+
import java.util.List;
17+
import java.util.Optional;
18+
19+
import static de.peeeq.wurstio.languageserver.ProjectConfigBuilder.FILE_NAME;
20+
21+
/**
22+
* CLI entry point for build-map that reuses the same backend map pipeline as the language server.
23+
*/
24+
public class CliBuildMap extends MapRequest {
25+
private final WurstGui gui;
26+
27+
public CliBuildMap(WFile workspaceRoot, Optional<File> map, List<String> compileArgs, Optional<String> wc3Path, WurstGui gui) {
28+
super(null, map, compileArgs, workspaceRoot, wc3Path, Optional.empty());
29+
this.gui = gui;
30+
}
31+
32+
@Override
33+
public Object execute(ModelManager modelManager) throws IOException {
34+
if (modelManager.hasErrors()) {
35+
throw new RequestFailedException(MessageType.Error, "Fix errors in your code before building a release.\n" + modelManager.getFirstErrorDescription());
36+
}
37+
38+
WurstProjectConfigData projectConfig = WurstProjectConfig.INSTANCE.loadProject(workspaceRoot.getFile().toPath().resolve(FILE_NAME));
39+
if (projectConfig == null) {
40+
throw new RequestFailedException(MessageType.Error, FILE_NAME + " file doesn't exist or is invalid. " +
41+
"Please install your project using grill or the wurst setup tool.");
42+
}
43+
44+
WLogger.info("cli buildMap " + map + " " + compileArgs);
45+
try {
46+
File targetMapFile = executeBuildMapPipeline(modelManager, gui, projectConfig);
47+
return targetMapFile.getAbsolutePath();
48+
} catch (CompileError e) {
49+
WLogger.info(e);
50+
throw new RequestFailedException(MessageType.Error, "A compilation error occurred when building the map:\n" + e);
51+
} catch (Exception e) {
52+
WLogger.warning("Exception occurred", e);
53+
throw new RequestFailedException(MessageType.Error, "An exception was thrown when building the map:\n" + e);
54+
} finally {
55+
if (gui.getErrorCount() == 0) {
56+
gui.sendFinished();
57+
}
58+
}
59+
}
60+
61+
@Override
62+
protected File ensureWritableBuildOutput(File targetFile, boolean isFinalWrite) {
63+
if (!targetFile.exists()) {
64+
return targetFile;
65+
}
66+
try (FileChannel ignored = FileChannel.open(targetFile.toPath(), StandardOpenOption.WRITE)) {
67+
return targetFile;
68+
} catch (IOException e) {
69+
throw new RequestFailedException(MessageType.Warning,
70+
"The output map file is in use and cannot be replaced: " + targetFile.getAbsolutePath());
71+
}
72+
}
73+
}

0 commit comments

Comments
 (0)