Skip to content
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.

Sign up for GitHub

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

Fix edge-case positional/named argument handling in image() #1627

Merged
merged 1 commit into from
Jan 21, 2024
Merged

Fix edge-case positional/named argument handling in image() #1627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions OpenDreamRuntime/DreamManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ public sealed partial class DreamManager {

// Global state that may not really (really really) belong here
public DreamValue[] Globals { get; set; } = Array.Empty<DreamValue>();
public List<string> GlobalNames { get; private set; } = new List<string>();
public List<string> GlobalNames { get; private set; } = new();
public Dictionary<DreamObject, int> ReferenceIDs { get; } = new();
public Dictionary<int, DreamObject> ReferenceIDsToDreamObject { get; } = new();
public HashSet<DreamObject> Clients { get; set; } = new();
public HashSet<DreamObject> Datums { get; set; } = new();
public Random Random { get; set; } = new();
public Dictionary<string, List<DreamObject>> Tags { get; set; } = new();
public DreamProc ImageConstructor, ImageFactoryProc;
private int _dreamObjectRefIdCounter;

private DreamCompiledJson _compiledJson;
Expand Down Expand Up @@ -123,8 +124,9 @@ public bool LoadJson(string? jsonPath) {
throw new FileNotFoundException("Interface DMF not found at "+Path.Join(rootPath,_compiledJson.Interface));

_objectTree.LoadJson(json);

DreamProcNative.SetupNativeProcs(_objectTree);
ImageConstructor = _objectTree.Image.ObjectDefinition.GetProc("New");
_objectTree.TryGetGlobalProc("image", out ImageFactoryProc!);

_dreamMapManager.Initialize();
WorldInstance = new DreamObjectWorld(_objectTree.World.ObjectDefinition);
Expand Down
25 changes: 23 additions & 2 deletions OpenDreamRuntime/Procs/DMProc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,14 +977,25 @@ public DreamProcArguments CreateProcArguments(ReadOnlySpan<DreamValue> values, D

var argumentCount = argumentStackSize / 2;
var arguments = new DreamValue[Math.Max(argumentCount, proc.ArgumentNames.Count)];
var skippingArg = false;
var isImageConstructor = proc == Proc.DreamManager.ImageConstructor ||
proc == Proc.DreamManager.ImageFactoryProc;

Array.Fill(arguments, DreamValue.Null);
for (int i = 0; i < argumentCount; i++) {
var key = values[i*2];
var value = values[i*2+1];

if (key.IsNull) {
arguments[i] = value;
// image() or new /image() will skip the loc arg if the second arg is a string
// Really don't like this but it's BYOND behavior
// Note that the way we're doing it leads to different argument placement when there are no named args
// Hopefully nothing depends on that though
// TODO: We aim to do sanity improvements in the future, yea? Big one here
if (isImageConstructor && i == 1 && value.Type == DreamValue.DreamValueType.String)
skippingArg = true;

arguments[skippingArg ? i + 1 : i] = value;
} else {
string argumentName = key.MustGetValueAsString();
int argumentIndex = proc.ArgumentNames.IndexOf(argumentName);
Expand All @@ -1005,6 +1016,9 @@ public DreamProcArguments CreateProcArguments(ReadOnlySpan<DreamValue> values, D

var listValues = argList.GetValues();
var arguments = new DreamValue[Math.Max(listValues.Count, proc.ArgumentNames.Count)];
var skippingArg = false;
var isImageConstructor = proc == Proc.DreamManager.ImageConstructor ||
proc == Proc.DreamManager.ImageFactoryProc;

Array.Fill(arguments, DreamValue.Null);
for (int i = 0; i < listValues.Count; i++) {
Expand All @@ -1020,8 +1034,15 @@ public DreamProcArguments CreateProcArguments(ReadOnlySpan<DreamValue> values, D

arguments[argumentIndex] = argList.GetValue(value);
} else { //Ordered argument
// image() or new /image() will skip the loc arg if the second arg is a string
// Really don't like this but it's BYOND behavior
// Note that the way we're doing it leads to different argument placement when there are no named args
// Hopefully nothing depends on that though
if (isImageConstructor && i == 1 && value.Type == DreamValue.DreamValueType.String)
skippingArg = true;

// TODO: Verify ordered args precede all named args
arguments[i] = value;
arguments[skippingArg ? i + 1 : i] = value;
}
}

Expand Down
Loading