BUG: Giving Worn Clothes to Actors Crashes Alan #57
Comments
|
Seems the 'give' verb must be modified with clothing items. In this code example, the hero gives the boxers to the assistant but when the hero examines the assistant, the assistant is already wearing the boxers, not just carrying them. I'll take a look.
-Anssi
________________________________
From: Tristano Ajmone <notifications@github.com>
Sent: Tuesday, January 29, 2019 11:14 AM
To: AnssiR66/AlanStdLib
Cc: Subscribed
Subject: [AnssiR66/AlanStdLib] BUG: Giving Worn Clothes to Actors Crashes Alan (#57)
I've uncovered a nasty bug affecting clothing, which causes Alan to crash:
SYSTEM ERROR: Stack is not empty in main loop
The crash was tested and confirmed with different Alan SDKs, both latest relea and dev snapshot:
* Alan v3.0 beta6
* Alan v3.0 beta6 build 1862
It affects both the StdLib and the Italian Library, and most likely it's a problem rooted in Alan which surfaces when using clothing features of the library.
The test files that show this bug can be found in commit 093e551<093e551> of the temporary clothing-tests-prep<https://github.com/AnssiR66/AlanStdLib/tree/clothing-tests-prep/tests> work branch:
* clothing.alan<https://github.com/AnssiR66/AlanStdLib/blob/a9b7f77/tests/clothing.alan>
* clothing_bug_crash.a3log<https://github.com/AnssiR66/AlanStdLib/blob/093e551/tests/clothing_bug_crash.a3log>
* clothing_bug_crash.a3sol<https://github.com/AnssiR66/AlanStdLib/blob/093e551/tests/clothing_bug_crash.a3sol>
The crash always occurs when examining an actor after having given him a clothing item that was being worn by the hero:
inventory
You are empty-handed. You are wearing your undershirt, your sneakers, your
boxers and your socks.
examine boxers
Just your boxer shorts (the loose type), white and plain.
; NOTE: boxers are currently bing worn by hero!
give boxers to assistant
(taking your boxers first)
You give your boxers to the assistant.
; Now comes the crash (using 3.0beta6):
examine assistant
She's your personal shopping assistant. The assistant is carrying your
boxers (being worn).
As you enter the twilight zone of Adventures, you stumble and fall to
your knees. In front of you, you can vaguely see the outlines of an
Adventure that never was.
SYSTEM ERROR: Stack is not empty in main loop
At source line 655 in 'clothing.alan':
WHEN hero AT dressing_room
<If you are the creator of this piece of Interactive Fiction, please help
debug this Alan system error. Collect *all* the sources, and, if
possible, an exact transcript of the commands that led to this error, in
a zip-file and send it to support@alanif.se. Thank you!>
The referenced line 655 in the error seems unrelated to the actuall problem, for this can be reproduced in any adventure created with the StdLib (English and Italian alike), so its cause seems related to the transfer of worn clothing items.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#57>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AF2lHtgHrkF2Q-Y5DuGLdnP4K1toenOPks5vIBD8gaJpZM4aXmgu>.
|
Notes on Approacing SolutionsOk, I want to jot down some notes here before tweaking the library code, for there are a variety of aspects to consider in handling transferal of worn items, so it's best to do some brainstorming first. I just want to make sure that we don't entangle things up, because there seem to be already quite a few places in the code that are handling/manipulating clothing! All links to library sources provided here will refer to the latest current commit (7a7b6d8), so they'll always reference the current status of the code. Also, I think that this might ultimately be an internal Alan bug. Even if we forgot to handle some attributes or moving clothing into/out of Do you think the bug is on Alan side, in the library, or both? Clothing StatusSo, we have to keep in account:
Fix ProposalsI'm pasting here you'r proposals from the Yahoo list email: Add Give Verb to Clothing Class
Here we need to consider carefully the possibility of the So, the safe bet here would be to use Here we must be careful about the Using EXTRACT to Fix Wearing/Donned
Using EXTRACT seems a good idea, for it would work whenever a clothing item is moved out of a container. So, my guess is that we could place it on the ACTOR class (which already has an EXTRACT to check compliancy) and safely assume that any clothing leaving an actor container should be considered undonned. My worries here are in case of verbs take are implicitly extracting the clothing item in order to make it worn by the Hero (or an NPC, in case of custom verbs by authors). Just want to make sure that the order of execution is such that even if the EXTRACT makes it undonned it won't interfere with the body of a verb that wants to make it donned (ie, that execution of a verb DOES block effectively comes in execution after the EXTRACT resulting from implicit taking). |
|
I've been able to reproduce the underlying root cause for the crash, which of course should not happen whatever a library does... It seems that it is the existence of the The I'll keep digging... |
I'm not sure I understand the full implications of this, but at least I do understand the area of the problem. Mhhh ... this is going to be tricky, for it clashes with the EXTRACT solution I had in mind (see below). Updated TestI've updated the test to make it show more details about the clothing item before and after it's given to the NPC:
So, it looks like the problem here is that after handing the boxers to the assistant there are still marked as For this reason, the First thing to do here is to ensure that worn items leaving any actor become undonned, no longer in Using EXTRACT(s) to Handle Worn Items?It looks like the cleanest solution to library problem would be to handle donned items via EXTRACT:
This approach would garantee correct clothing handling even with custom author verbs — whereas leveraging only the But there is another problem here too, the VERB Personally, I think that the |
|
To remove the extraneous manipulation of list seems reasonable, if for nothing else to clean up the code. (This might be a bit out of scope and perhaps belongs somewhere else, alanif-group? manual? Please quote me or copy this there if you see a better place for it.) I think the broader implication here is that Alan was created as a language to very closely "mimic", "mirror" the player level of narration. The deeper you go into using Alan as a general Turing-complete programming language, which it seems some parts of the library are close to doing here, the trickier it will be to get around limits imposed by that initial design idea. E.g. Alan has containers, containers inherently have limits and extract checks on them so that the player should not attempt to add or remove stuff that the author did not want him to be able to do. But if the Alan "program" starts to manipulate, explicit or implicitly those containers, that abstraction, if you will, will start to break. The "program" might really want to move things in and out of containers without those rules applying, or maybe they should. There is no way to tell. And we don't want two statements for manipulation of containers. Having said that, I think there is no other option than being aware of the "execution context" somehow. (There are actually two problems here, one is the bug that happens since the Extract check jumps right out of the execution and leaves an unhandled stack, the other, which is the focus of this discussion, how should/could Extract be handled in "program" code.) In the current example, what is the expected player experience? As the Maybe the library has to cater for these cases by "knowing" which things to not move out of other containers. As I'm not too familiar with the deep logic of the library, I'm unsure what the strategy is, on how moving things belonging to other people ended up in an event. So, please enlighten me, although there is probably a lot of background on the library design... |
|
Ciao Thomas,
Not at all, for it actually brings up the question of why the library currently needs resorting to these type of parallel and cross item "hacks" to achieve this, so to speak. I think that the nature of the problem is, as you pointed out, the separation between player commands and internal actions of the adventure. In this respect, one missing feature in Alan seems to be ability to trigger a VERB from within the code, so that an action could be carried out as if invoked by the player. For example, implicit taking could be carried out by the "give" verb by triggering the "take" verb. This is actually the limitation we are facing in this very context, for we have no way of implicitly disrobing a clothing item in the course of the "give" verb (or any verb that would move the item away from the actor's wearing it). So, probably, if Alan were to allow invoking verbs from within code the whole codebase of the library would be rather different. Of course, triggering verbs from code might also introduce new problems (eg. should these be executed in "silent" mode regard messages they might produce?).
This is a question which should be discussed with whole community, for it's always delicate when it comes to tinkering with Alan behavior. One of the great things about Alan is that it allows total freedom in creating adventures by providing a clean-slate approach to the adventure world. No other IF authoring system allows this, the only exception being TADS — but then, since in TADS the grammar parser is part of the Standard Library, a clean-slate approach would require high expertise to be put into practice. The problem with libraries is that they tend to limitate that freedom in a manner which is directly proportional to the amount of features and complexity they introduce. So, if on the one hand a library simplifies authoring, on the other it also imposes a "philosophy" on how an adventure should be structured/desgined. But I think that the StdLib has always been clear about this, and it always encouraged authors to customize it, and use it as a starting point rather than a fixed means. So, what might seem reasonable in the context of the StdLib might not be seen as something good by free-form authors who have different approaches the adventure world. But to answer your original question:
Handling moving items ended up in EVENTs because Alan doesn't provide a means to trigger VERBs from code. If VERBs were executable from the library code (like they are in many IF systems) then implicit actions would all go through the same code, and we wouldn't have to deal with out-of-synch elements that need to be intercepted at various points. Before "giving" a clothing items to another actor, we could "remove" it implicitly, and then all the attributes would be fine. At least, that is my understanding of this situation, having studied the code, but I can't speak for Anssi of course. |
|
Thank you, for that explanation. I'm sure there is more to the "event strategy" than being able to trigger verbs from code, because you can always duplicate the code of the verb, right? Granted, that would be a lot of duplication, so I'm not saying that the library should do that instead, just trying to figure out what is, or are, the root problems here. Perhaps duplication is one reason to move this code to an event, I know there has been discussions about a "procedure" that you can call, and I think the current best thinking is to use an event for this. When you say, trigger Verb, which part of the verb handling are you actually after? The Checks? The resolution through the class inheritance tree? The additive behaviour of Checks and Does clauses? Or is it just a few lines in the specific Does clause of the basic "Take" verb that you want to call? |
|
I would also add to the above considerations: The problem is not so much ensuring that library verbs manipulate attributes correctly — that would be easy to achieve, we'd only need to replicate the code from The problem is ensuring that custom author verbs added to an adventure won't break up handling of special classes (like clothes). The whole idea is to take off the burden from authors by having the library handle in the background chores related to the classes it introduces. Let's say an author wants to add a new verb "tear clothes" (eg. as an act of keriah in a Jewish funeral). This action implies removing the clothing item, and if the author forgets to do it, the item will remain worn by the Hero (or the NPC) and in his Forcing authors to have to handle library internals would impose quite a burden on end users, which kind of defies the whole purpose of using a library to simplify authoring. Of course, at times this is inevitable with specialized classes with complex behavior. But, whenever possible, the library tries to handle these chores in a manner that is safe in view of (unknown) custom verbs. EVENTs are one the most viable tools to achieve this, lacking invocation of VERBs from code. Another feature which might help here would be having "functions", so to speak, ie. block of code that could be invoked anywhere from code for immediate execution. That would ensure reusability of snippets across the library, and that the order of execution of those snippets is controllable (unlike Events, which could end up executing in out-of-synch order). Informing author about the need to invoke specific code "functions" when handling entities of special classes would not be a huge burden. Since this functions would be sharable by library- and user-code, they would allow to break up VERBs in smaller chunks and avoid redundancy of code. If functions were to introduce to much of a complexity, then having MACROS to replicate shared code could be a viable alternative (define once, use anywhere), although they would add some overhead in code/memory size. I think that this problem of snippets reusability (whether by allowing invoking verbs from code, or introducing functions or macros) is a general problem that affects not only the library but Alan adventures in general, and I've seen the issue come up more than once in features requests. Of course, in a big codebase like the StdLib, which also aims to be extensible by end users, this problem is felt with more urge than free form adventures. |
That's difficult to answer. And probably having aribtrarily executable code snippets would be an easier solution, for then we'd only need to move the shared parts of a VERB into such "functions" (or MACROS). I'd be happy with having MACROS, not really worried about code duplication at all (memory not being a real issue nowadays), and probably macros would be easier to implement via some preprocessor, instead of altering Alan internals. Something like: and then the macro could be usable anywhere by just mentioning it's ID: or some other syntax (eg: |
|
Good point, with being "immune" to author additions. But doesn't that again bring up the question on how to proceed when the "action" aborts, like is/was the case with Extract. If the player is "calling" the Verb a failed check should, and I think, must abort the execution. It would be very strange if the execution continued with other pending statements in this case. But if a library does some manipulation, calls a Verb and the check fails, should that be allowed to continue? Otherwise the world might be in disarray when the library thought it was finished. That sounds like standard exception handling. Perhaps that is a thought to explore. Again, two issues, function/macros/snippets, and handling aborted execution from "calling a Extract/verb/function/macro/snippet". |
|
Well, now that you mentioned the issue of aborting VERBs execution, I might add to this that currently the situation with VERBs Let's say I need various classes to execute some As for MACROS, I'd say that it's implicit that author should use them wisely, and only as a means to control same code in different places by a single definition point. The idea of a macro would be twofold:
Obviously, the assumption here is that every macro should be created with a specific use in mind, and that they'd be safe to use only in the intended context. I was thinking that a macro could be used to handle attributes, etc. But of course they need to rely on parameters names which are shared across the places where the macro will be used (eg. Some languages (eg. Fasm assembly) provide complex macros systems, allowing parameters and nested macros, conditional branching macro definitions, etc. But these would probably be to complex to use, as well as implement. FUNCTIONs, on the other hand, would probably allow using parameters in a more explicit way when invoked, so they could handle some cases that macros can't. Eg., I'm not saying any of this is simple to implement, but definitely this is an area of Alan which is worth addressing with one solution or another (probably there are simpler solutions, I just can't think of any). |
|
Hi, popping into the conversation in the middle but anyway : )
I will look into the clothing code as soon as possible. There should be a way to make the clothing items to be transferred to the recipient correctly without too much hassle. I mentioned Extract in passing, because that came to mind, but there might be simpler ways. And especially if it conflicts with the way Alan is meant to work, like Thomas mentioned, we can find alternative routes.
I would keep the "(being worn)" for now, because it is especially needed when examining NPCs:
x man
The man is carrying a bag, a key and a grey suit (being worn).
The library distinguishes only between what the *hero* is carrying or wearing:
x me
You are carrying a bag and a key. You are wearing a grey suit.
but with NPCs the description only mentions *carrying*. With the hero, the "being worn" should never show, so if that happens, it needs fixing.
The above as a short comment on the points you took up.
As to functions:
The closest way that it is possible at present to imitate functions is to make an instance and give it a description:
THE remove_clothes ISA THING
DESCRIPTION
FOR EACH cl ISA CLOTHING, IN hero
DO MAKE cl NOT donned.
EXCLUDE cl FROM wearing OF hero.
EMPTY worn.
END FOR.
END THE.
and then call this 'function' by "DESCRIBE remove_clothes." etc.
But here it is difficult to refer to the exact object the player referenced, it can only be on a general level, like above when removing all clothes, etc... as far as I can see.
(it could even be possible to say "Every Function Isa Thing", and use functions that way (The remove_clothes Isa Function) if you don't want to use 'thing' etc.)
I'll experiment for my part and let you know if there is something I can present as a solution for the transfer of clothing items...
…-Anssi
From: Tristano Ajmone <notifications@github.com>
Sent: Wednesday, January 30, 2019 1:45 PM
To: AnssiR66/AlanStdLib
Cc: AnssiR66; Comment
Subject: Re: [AnssiR66/AlanStdLib] BUG: Giving Worn Clothes to Actors Crashes Alan (#57)
Well, now that you mentioned the issue of aborting VERBs execution, I might add to this that currently the situation with VERBs DOES [AFTER|BEFORE] is lacking some way to explicitly abort.
Let's say I need various classes to execute some DOES AFTER code after a standard verb, just to handle some extra bits but without having to rewrite the whole code from the main verb's DOES block. Currently there isn't a way to abort the execution chain of VERBs. Either one uses a DOES ONLY, or the whole chain of AFTER verbs will be executed in heritance order.
As for MACROS, I'd say that it's implicit that author should use them wisely, and only as a means to control same code in different places by a single definition point. The idea of a macro would be twofold:
* allow easy reusabilty of code snippets defined in a single place.
* provide end users with library defined macros which they can safely use to inject shared code snippets in their own VERBs, EVENTs, etc. Even if library updates modify the code in the snippet, these should still work in end user code.
Obviously, the assumption here is that every macro should be created with a specific use in mind, and that they'd be safe to use only in the intended context.
I was thinking that a macro could be used to handle attributes, etc. But of course they need to rely on parameters names which are shared across the places where the macro will be used (eg. THIS or obj). Probably this would not provide a universal solution, but this is a genral problem with macros.
Some languages (eg. Fasm assembly) provide complex macros systems, allowing parameters and nested macros, conditional branching macro definitions, etc. But these would probably be to complex to use, as well as implement.
FUNCTIONs, on the other hand, would probably allow using parameters in a more explicit way when invoked, so they could handle some cases that macros can't. Eg., THIS inside the function would refer to its parameter of invocation, allowing them to be used to reference different instance during execution.
I'm not saying any of this is simple to implement, but definitely this is an area of Alan which is worth addressing with one solution or another (probably there are simpler solutions, I just can't think of any).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<#57 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AF2lHr33fpHBsbF1pmWbDiFBczo_Rph5ks5vIYXRgaJpZM4aXmgu>.
|
|
[ LINKS UPDATED AFTER BRANCH DELETION AND MERGE ] Anssi, I've update the tests to check how the But further tests also show that the We need to start checking and fixing various clothing specific verbs before attempting changes to solve the issue relating to the crash bug, otherwise things might get out of control. There are too many cross factors at play here, and we need to get straight the basic verbs first. |
Corrupt Memory Bug?An update to the tests reveals a new non-crashing bug related to clothing: Where, initially (line 49), trying to remove a non existent item produces the expect respons from ARun: But after moving around some worn items (not properly handled) the same command produces no response (line 148): I'm not sure wether it's related to the same bug of EVENTs/EXTRACT, but I though of pointing it out. |
Ampia la rosa dei test sul vestiario (in EGA) per coprire vari bug e problemi individuati e discussi nella libreria upstream: - AnssiR66/AlanStdLib#57 - AnssiR66/AlanStdLib#58 - AnssiR66/AlanStdLib#60 - AnssiR66/AlanStdLib#61
|
I've been running tests with the latest developer snaphshot Alan 3.0beta6 build 1866 (in the Italian library I'm now using alway latest snapshots to work) and noticed that the bug crash has now shifted place (it occurs in other point of the source code); so chances are that this problem might actually be solved. I'm looked into the Alan commits history and have noticed that @thoni56 has been tackling this issue in various commits, trying to unwind the problem by changing various parts of Alan source. So, a bit thank to Thomas for all the hard work, and let's see if this gets solved so we can use Events to handle clothing. |
|
Yes, rather than keeping up with the conversation ;-) I have been working on understanding the core problem. And as I stated earlier, I think, it is the effect of the "thrown Extract" that leaves an unhandled stack, which in this particular context contains the loop variables for the "rest" of the iterations for manipulating the clothing containers. I'm in no manner close to a solution, because I think it is a principle one. And I have no easy fix for that. (The case that the Extract should prevent and abort a player action of removing something from a container that shouldn't be allowed. And that we in this case want to do that "silently".) If I just think about the logic here I would investigate a strategy where the library first tried to "get" the item if possible, which might abort by means of an Extract, and once that is done do all the manipulations using sets as an intermediary storage, sets not being possible to prevent, restrict or abort movement in and out of. This does not solve the primary problem, what to do with an unhandled stack when returning to the main loop. I suspect that some games silently just have been running with a larger and larger unhandled stack. (Not that that is a real problem since each "layer" only takes a handful of bytes...) |
|
Thanks for the insight @thoni56. unfortunately, since the AMachine is undocumented I have no idea of how these internals work. I imagine that some Alan keywords behave like functions, having a stack frame for their execution. If I've understood correctly, the nature of the problem here is that the Event and the Extract don't share a common context (references to instances, etc.) and that there isn't an underlying system in AMachine to handle failing executions (in this case, for the Event to know that the Locate failed due to Extract preventing it)? Or is just that unexpected cross-triggerings are causing mishandled stacks which are not being accounted for? I'm trying here to think in terms of the Glulx VM, which I've studied a bit and have some idea of how it handles such cases — Glulx is the only model I can rely to in this context, for AMachine is a black box to me. |
|
The AMachine is a stack machine pushing values on the stack which are then "ate" by the next instruction. Also local and loop variables are also kept on the stack in "frames". A nested loop would cause two frames to be used and some data space for the loop variables. If, as in this case, an instruction to move an instance from a container triggers an Extract the valuation of the Extract is done "on top" of the current stack. If nothing exceptional happens, the Extract will execute and exit normally, causing the next iterations of the innermost loop to be executed until that loop is executed and the inner "frame" popped of. If the outer loop is not also finished that "frame" will also be popped and the stack should be empty when, e.g., the event terminates. What happens is that the extract "aborts" and does not return, but skips all the pending loops and returns to the main player loop without terminating/removing/handling the two "frames". The issue here is should this ever happen? It is possible to just throw the pending frames away when aborting. But is this the right thing to do? Yes, probably in some cases, and no, in other cases. It feels that the "clothing" problem is an instance that we would it more "correct" to actually keep looping and not aborting. But if it is the player that is more directly responsible for the removal from the container, then we probably would consider an abort the correct option. I would probably have to learn more about what exactly the There is a problem with the contexts of Extract, I know we had conversations about this in the past. I think that discussion got lost, so I'll have to think that through again to find a solution (and probably a fix) to access command parameters as well as the current actor, and both the instance being removed and the one being removed from. (I think the latter was the problem, $1 can only hold one of these, I wonder what |
|
Thanks for the detailed reply @thoni56! I'll have to read it thouroughly later on, because my dog is sick today and keeps asking me to take him out every 2 hours ... so today I'be on-and-off the PC like a yoyo.
I've tested that, and using I wonder if the |
|
Thanks. So
The problem with parameters is that it is impossible for the compiler to know which verb was executed to trigger the Extract. In fact, as we have seen in the original issue, it might not even be a verb that trigger the extract. So, no, Alan is built on the notion that the compiler should be able to know enough about the execution context to make through analysis possible. I'm starting to think that |
I think that keeping Extract is fine, as long as end users have a clear picture of its intended use and limits. Also, it's quite conceivable that the compiler won't be able to catch every possible compile time error, and warning and error message usually provide a good hint on the nature of the problem. Again, I think that if users are trying to use some Alan keywords beyond their originally intended context is to workaround the lack of an explicit way to execute verbs (I'm thinking here of Inform's way to use |
This commit squashes the `dev-clothing` branch which introduces the new clothing system that solves all the problems relating to clothing. The list of changes is very long, but to summarize: - Removed `worn` entity and and `wearing` set; now replaced by a simple `worn` boolean (defined on `thing`). - New `facecover` layer/attribute. - Uncostrained Layers (no longer exponentially enumerated). - Special clothings like skirts and coats are no longer bound to fixed layers but can be assigned to any layer via (new) special attributes. - Improved Library messages involving clothing. - General improvements in the behavior of all verbs that might involve clothing items. Closes #52. Fixes #57. Resolves #58. Fixes #60. Fixes #61. Resolves #64. Closes #65.
This commit squashes the `dev-clothing` branch which introduces the new clothing system that solves all the problems relating to clothing. The list of changes is very long, but to summarize: - Removed `worn` entity and and `wearing` set; now replaced by a simple `worn` boolean (defined on `thing`). - New `facecover` layer/attribute. - Uncostrained Layers (no longer exponentially enumerated). - Special clothings like skirts and coats are no longer bound to fixed layers but can be assigned to any layer via (new) special attributes. - Improved Library messages involving clothing. - General improvements in the behavior of all verbs that might involve clothing items. Closes #52. Fixes #57. Resolves #58. Fixes #60. Fixes #61. Resolves #64. Closes #65.
[ LINKS UPDATED AFTER BRANCH DELETION AND MERGE ]
I've uncovered a nasty bug affecting clothing, which causes Alan to crash:
The crash was tested and confirmed with different Alan SDKs, both latest relea and dev snapshot:
It affects both the StdLib and the Italian Library, and most likely it's a problem rooted in Alan which surfaces when using clothing features of the library.
The test files that show this bug can be found in commit 2e5f8c3
093e551 of the temporary:clothing-tests-prepwork branchega.alan—clothing.alanbug_crash.a3log—clothing_bug_crash.a3logThe crash always occurs when examining an actor after having given him a clothing item that was being worn by the hero:
The referenced line 655 in the error seems unrelated to the actuall problem, for this can be reproduced in any adventure created with the StdLib (English and Italian alike), so its cause seems related to the transfer of worn clothing items.
The text was updated successfully, but these errors were encountered: