nodebox / seed

Procedural Content Generator
https://seed.emrg.be/
MIT License
22 stars 8 forks source link

Before running the sketch, check if all references are valid #17

Open fdb opened 6 years ago

fdb commented 6 years ago

Currently resolving references happens at runtime, which means sketches like this only fail 50% of the time:

root:
- This line is fine
- This line contains an invalid {{ reference }}

To avoid these errors happening at runtime I suggest creating a validation step in parsePhraseBook that checks if all references in the script are valid.

kunal-mohta commented 6 years ago

This would require another loop to be run after the already running loop, because once the first loop is completed then only we can determine whether a reference is defined or not. If I am going in the right direction, can I try this?

fdb commented 6 years ago

Actually we just need to check all references we encounter during parsing. We don't need to run the code "normally" at all.

Give it a try if you want!

kunal-mohta commented 6 years ago

Okay, I will give it a try.

kunal-mohta commented 6 years ago

Wait, I don't think I got this right. Say for example, we have

root:
- This is {{ reference1 }} //(*)
- This is {{ reference2 }} //(**)

//(***)
reference1:
- Some stuff

reference3:
- Some stuff

How will we know at line (*) and (**) that reference1 and reference2 are defined after (***) or not, without running the code "normally"

stebanos commented 6 years ago

Because before running the code we already have the phrase book, and a syntax tree through which all of the code runs. After parsing the code, the syntax tree needs to be traversed and every time a key is encountered (node.type === NODE_KEY) it should be checked against the phrase book. One caveat though, this key could also refer to an argument that has been passed, or an import key.

2018-02-21 7:02 GMT+01:00 k-cap notifications@github.com:

Wait, I don't think I got this right. Say for example, we have

root:

  • This is {{ reference1 }} //(*)
  • This is {{ reference2 }} //(**)

//(***) reference1:

  • Some stuff

reference3:

  • Some stuff

How will we know at line () and () that reference1 and reference2 are defined after () or not, without running the code "normally"

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodebox/seed/issues/17#issuecomment-367222942, or mute the thread https://github.com/notifications/unsubscribe-auth/AADAvgPZ8ch4GrDdGM8xhLtBwC4gR7PUks5tW7F6gaJpZM4SKxjQ .

kunal-mohta commented 6 years ago

Oh! Okay. I thought you were planning to implement this inside parsePharsebook() function.

kunal-mohta commented 6 years ago

Should the check be included here

fdb commented 6 years ago

No idea. @stebanos ?

stebanos commented 6 years ago

No, it should be checked after everything is parsed, before returning the phraseBook, so yes, inside the parsePhrasebook function, but at the end. Because it is only at this point we know for sure what all the phrases and references are.

kunal-mohta commented 6 years ago

Won't that mean rewriting the code that extracts the references from {{ ... }}, that already has been written?

stebanos commented 6 years ago

No, the check needs to perform a similar action as what happens in the Interpreter class. For each phrase, the Parser class returns a tree of nodes of a certain type, some of these nodes are of type NODE_KEY, it's these ones that need to be checked against the whole of the phraseBook. Since these nodes are "buried" within the tree, the tree needs to be traversed, like what happens in the visit(node) function.