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.

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

Added URL to component and field type mismatch error messages #1184

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

droqen
Copy link

@droqen droqen commented Nov 23, 2023

My attempt to resolve #1183
I haven't successfully built Ambient from this, I tried and it took a super long time... but I do expect it to work. The change is very small.

@droqen droqen requested a review from philpax November 23, 2023 17:46
@droqen
Copy link
Author

droqen commented Nov 23, 2023

Rather than give a list of all types (very long, dynamic), I put a link in the error message to the relevant section in the book (shorter, static, but might change). Is that something we're avoiding doing for some reason?

The intended (untested) error message would look like this:

Error: Failed to resolve dependencies for pre-build

Caused by:
    Failed to resolve type `Usize` for component `boid_neighbour_count`
    See https://ambientrun.github.io/Ambient/reference/package.html#valuetype for valid types

@philpax
Copy link
Contributor

philpax commented Nov 23, 2023

Looks good to me, but we might want to use https://ambient.run/docs/reference/package instead. However... that might be out of date (it's not updated automatically with the repo - but we probably want versioning instead), and more annoyingly, it doesn't have anchors (https://github.com/AmbientRun/Web/issues/47).

@FredrikNoren Have we decided where the docs should live? If it's ambient.run then I'll see if I can fix up the anchors issue and we'll deal with versioning later I suppose

@philpax
Copy link
Contributor

philpax commented Nov 23, 2023

Ok, thought about it some more, we should use ambient.run. I've fixed #47, so you should be able to replace the URL with https://ambient.run/docs/reference/package#valuetype - once you've done that, feel free to merge it!

EDIT: Okay I just actually tested that link and the anchor routing is broken 🤦 Let me see...

EDIT2: Yeah no idea, I think the anchor's being stripped off by the routing or something

@droqen
Copy link
Author

droqen commented Nov 24, 2023

Oops, I missed your edit. I made an issue for it, haha. #1191

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.

Request: More didactic error message when invalid type in ambient.toml
2 participants