Code review request - Githubissues

jonkiparsky / z-engine

Automatically exported from code.google.com/p/z-engine

0 stars 0 forks source link

Code review request #12

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago

Purpose of code changes on this branch:

When reviewing my code changes, please focus on:

This repackaging splits the project into two packages, one for core and one for 
game files. Does this look easier to modify, from the designer's perspective?

Original issue reported on code.google.com by jon.kipa...@gmail.com on 13 Aug 2011 at 6:19

GoogleCodeExporter commented 8 years ago

Looks okay to me. Will splitting Nouns in to it's own package make sense? So 
Items/Objects would be separated, like rooms. Could likely do the same for 
verbs and prepositionals, just to make it a little cleaner, but it's up to you.

Original comment by i.Snowbl...@gmail.com on 23 Aug 2011 at 12:12

Changed state: Accepted

GoogleCodeExporter commented 8 years ago

Also, would it not make sense to move the core grammar objects in to zengine? 
Such as None, Take, Drop, Go, etc.

Original comment by i.Snowbl...@gmail.com on 23 Aug 2011 at 12:40

GoogleCodeExporter commented 8 years ago

Separate packages could make sense, but it would be a radical piece of surgery 
to the Parser. Instead of looking at what you get and deciding what it is, 
you'd be looking in a given package and deciding whether what you've got is one 
of those. Not necessarily a bad idea, in fact it might be quite a good idea, 
but it would really rip up a lot of the existing code, and I don't know if 
that's what I want to do right now. It might be that it comes along with 
parsing by phrases, which I know you're interested in seeing. 

As for the grammar items, I'd like to leave all of the verbs in one place. And 
since None is parsed like a grammar item, I'd like it to live with the other 
grammar items.

Original comment by jon.kipa...@gmail.com on 23 Aug 2011 at 3:40

GoogleCodeExporter commented 8 years ago

It wouldn't be that big of a change, I have it working in the other branch, 
where I split grammar items in to Noun, Verb and Preposition packages, if you'd 
like to have a look at how it's done there.

Original comment by i.Snowbl...@gmail.com on 23 Aug 2011 at 11:41

GoogleCodeExporter commented 8 years ago

Original comment by jon.kipa...@gmail.com on 29 Aug 2011 at 4:07

Changed state: Fixed