Editor: test missing event handlers as a post-compilation step #2556
Conversation
This makes search much more reliable, as previously existing search for exact string could fail on any tiny difference or extra whitespace.
Here's a problem: InsideStringOrComment relies on scintilla styling. But styling is not necessarily present at the time if we just loaded a document when zoom-in to a function. So we force the styling in case it was not done here yet. Style only the portion of a document from the last styled position to the position of the found match (this speeds things up a little).
ivan-mogilko
added
what: editor
|
Hmm, maybe I should switch to searching preprocessed scripts variant. There's another reason to do this: if we like to have this scan as a distinct operation, which may be run on its own without compiling scripts. |
|
It could preprocess, and then search on it using a regex pattern, since ags script doesn't support nested functions it should be possible to find the functions, possibly with some checks to allow both void and function (maybe int too?) event handlers. The regex pattern can check for either Erh, maybe something like |
@ericoporto this is literally what I did in the first commit of this pr. I made this work for "zoom into file" function, but same method may be used for a global scan. |
ivan-mogilko
force-pushed
the
362--testmissinginteractions
branch
from
7e41443
to
df1afb8
Compare
|
Made it so that, if function's name was in the event property, but function itself did not exist, then pressing "..." button would create that function. |
|
No, this is not right, I was not thinking straight apparently. The function match should not be done over a script text, because that means regex search done over full script text for each event function name. Even if script is preprocessed, that will be very slow. The function match must be done over prepared list of functions (or specifically Set, or ordered list for more optimization). But in the end this must be a set of function names, which in turn we use to match event properties. |
|
About the function list, it's there already in the script autocomplete cache and used to fill the combobox at top of the script editor. Perhaps the script autocomplete cache should be kept somewhere when the script is unloaded so it can still be looked up? There is a discussion about an idea of a dictionary to store the autocomplete stuff in #1454 (it's in a different context, but perhaps there is something that would work for both cases). Alternatively, I guess looking only at the opened scripts would already be somewhat fine? As probably if you just wrote something you didn't close the script before hitting play to test what you just did. About actually testing this, what I noticed is that in the case of a room, if I have an invalid room event handler and click the play button for testing, it appears that it isn't checked - I guess because the room isn't rebuilt? Not sure. But if I click rebuild all scripts then the handler errors actually appear in the output panel. Edit: actually, it also happens with button onclick event handlers too, in any script. Edit2: I think for any handlers really, just a rebuild seems to trigger showing the warning messages but not clicking Play button. |
That's a good idea on its own, and I also wonder if it could have been a design mistake to attach AutoCompleteData directly to a Script object. It could be a dictionary of autocomplete caches, where script name is the key, stored separately and provided through a different interface. Also it could be called not AutoCompleteData, but in a more generic way, like ParsedScriptData, for example, since it's not only useful for autocomplete. Furthermore, it appears to me, that instead of having own script parser duplicated exclusively for autocomplete, this data could be generated by a script compiler's parser, since it's probably most advanced parser available here already. If there were a way to run compiler in "parsing only" mode, which returns a list of symbols. I don't know if it would be feasible to do with the old compiler, but new compiler seems to have components separated well. IMO it makes sense to consider reworking autocomplete in this vein, the interface kept more or less same (also for backwards compatibility), but implementation use script compiler under the hood. On the practical approach, even if the AutoCompleteData is not ready, it may be forced to generate. If this test is run separately, as a action from a menu, then of course it might force auto complete to rebuild and get its results. EDIT: Hmm, actually, looking around the Editor's code, it seems like autocomplete is regenerated on game launch already. So maybe it is available even if script was never opened for edit during this session. I'm going to check that out...
More importantly than being rebuilt -- the rooms must be loaded into memory in order to know about the event handlers.
Yes, because I added its run to post-compilation step only so far. UPD: I am going to edit engine warnings for missing handlers too, that might help noticing this at runtime as well. |
|
About the parser, I am not completely sure, but I think that the one for autocomplete data can pickup if you haven't finished writing some code instead of giving error and also understands that you are local to a function - so it shows local stuff, from picking up where the cursor is. The one in the compiler errors when the code is unfinished, and I don't know if it has any sort of interface to be able to pickup local symbols too - which are necessary for autocomplete. |
I suppose that might be a matter of making parser have its own interface exposed, and work in certain way, like, ignore unfinished parts, ignore all function bodies except one and gather local symbols from a function found at the given line, and so forth. Given that logic of parsing the code and gathering symbols seems to be the same, I think it is worth investigating this approach, and see how feasible it is to share same tool for multiple purposes. That's not something I want to try right away, but something to consider as an option in the future. |
ivan-mogilko
force-pushed
the
362--testmissinginteractions
branch
2 times, most recently
from
fb12375
to
7150670
Compare
* Added TestGameScriptIntegrity event to the Editor. * As a post-compilation step, Editor updates AutoComplete cache for all script modules (as necessary) and fires TestGameScriptIntegrity event. * Certain Components subscribe to TestGameScriptIntegrity, and run event handler checks for their respective types of game objects, using prepared AutoComplete data.
Do this as a post-room-compilation step.
ivan-mogilko
force-pushed
the
362--testmissinginteractions
branch
3 times, most recently
from
73a049e
to
3f46a5b
Compare
Add Enabled field to the InteractionEvents's members. Record call result at runtime and disable event handlers that were not found in script. Issue a warning for the first time when this happened. NOTE: Room events will likely be reported again when returning to the same room, as the state of event handler is not saved when room gets unloaded.
ivan-mogilko
force-pushed
the
362--testmissinginteractions
branch
from
3f46a5b
to
881d319
Compare
|
I changed it to use Autocomplete, and also put this process into a function called |
|
Okay, I merged this, as this works overall. |
Resolves #1363
This PR's additions may be separated in 3 parts:
function functionName(, and thus could break by simply an extra space. I changed that to using regex pattern, which finds bothfunctionandvoid, and handles whitespace. This keeps working even if you split function header on multiple lines, like:In addition, I tried to make it skip found matches if they are inside comments.
I also considered testing preprocessor conditions too, but on second thought decided to leave that be, as that would become tough to do without a full script's preprocessing, which seemed like an overkill for "..." button in events (I'd have to cache results and run preprocessing in background thread, or something...).
If a function's name is inserted into event property, but that function is not actually exist in script, then pressing "..." will create that function anyway.
Test missing event functions as a script compilation post-step. For this editor keeps a dictionary of preprocessed script texts at compilation stage, so that components could searching for regex patterns there and find matching functions.