Skip to content

Lingo fixes, better error messages, some other things#74

Merged
igorlira merged 11 commits intoigorlira:mainfrom
zdimension:pr1
Mar 22, 2026
Merged

Lingo fixes, better error messages, some other things#74
igorlira merged 11 commits intoigorlira:mainfrom
zdimension:pr1

Conversation

@zdimension
Copy link
Contributor

See list of commits.

Comment on lines +230 to +239
Err(ScriptError::new(format!(
"Cannot get property {} found on script instance {}. Valid properties are: {}",
warn!("Cannot get property {} found on script instance {}. Valid properties are: {}",
prop_name,
format_concrete_datum(
&Datum::ScriptInstanceRef(script_instance_ref.clone()),
player
),
valid_props.iter().join(", ")
)))
);
// documentation says getaProp should return VOID for nonexistent properties
Ok(DatumRef::Void)
Copy link
Owner

@igorlira igorlira Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a function that returns Void if the property doesn't exist: script_get_prop_opt, and there are cases in which trying to access a nonexistent property throws an error, such as when calling script.unknown_property directly or script.getProp('unknown_property'). In other words, getaProp should call script_get_prop_opt (it is currently calling script_get_prop)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was fixed in b2fba75.

@zdimension
Copy link
Contributor Author

@igorlira is there anything left to do on this PR?

Copy link
Owner

@igorlira igorlira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for sending this!

@igorlira igorlira merged commit f951d94 into igorlira:main Mar 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants