search

alan-if / alan

ALAN IF compilers and interpreters
https://alanif.se
Other
18 stars 3 forks source link

Buggy Behaviour with Limit Attributes? #35

Closed tajmone closed 3 years ago

tajmone commented 3 years ago

In test_indirect-containment.a3t you'll find a test which reveals inconsistent behaviour with attributes limits.

In the example, the Hero can carry max 20 units of weight; in the room every object weighs 10 units, so he can't carry more than two of those objects.

But there's a bag which is a weightless container without weight carrying limits.

If the hero puts 3 objects in the bag and then picks it up, ALAN doesn't complaint about the fact that the bags contents weigh more than the limit.

On the other hand, if the hero is carrying the empty bag, and tries to put those same three objects into it, ALAN won't allow the third one since the limit was already reached.

I seem to understand (from other tests in that file) that when it comes to items counts limits, a container will always count as a single item, regardless of its contents (e.g. the hero can carry only 3 objects, but one of them can be a bag with 20 objects).

I'm not sure if this is intentional by design (e.g. to allow carry-alls), but when it comes to weight (i.e. any attributes for limits) it makes little sense to count just an item's weight without the weight of its contents. So I'm not sure how the above inconsistency should have resolved.

thoni56 commented 3 years ago

I quickly fixed #34, but did not have time to check if that fixes this to. I'll be able to do that tonight.

I might not have been so quick to find what the problem was unless I had read up on the container (worn) issues discussed at lenght. I'll get back to this.

thoni56 commented 3 years ago

Oh, right. The crashes here was actually not related to the "bug" you described. Well, at least the crashes seems to be gone with that fix.

On the matter of aggregating attributes in a container when considering limits, a simplistic approach would be to just recursively sum up the weights. But the matter is actually somewhat more complicated. For weight it is natural to think about aggregation, but what about size? For that attribute the largest size should probably considered, not the sum, but it might also depend on the authors intention with the "size" attribute and how that relates to items being in a container.

Again, for size, items on the top level might probably add up, but items on the second level, do they even contribute to the size of the container?

There is currently no way to describe how a limit should consider container attributes, so intentional or not, I think it might be the only feasible, admittedly not perfect, option. Or do you have a suggestion?

My thoughts go again, to use explicit management of items in other items using sets and re-implement the container logic, limits and all. With that authors have full control over how attributes aggregates when removing things from a "container".

So I would consider container to be the language level simplified container mechanism with straight forward logic/semantic which also gives particular quirks and flaws when trying to use them beyond that. But they can be used as a starting point, giving language support for a container feature. For programmatic extensions of that feature I would probably look at replacing them with sets. Mind you, I haven't tried it so I don't know what problems would show up.

As I have mentioned before I think the worn-problem could be solved with a set attribute on the hero. This should also be easy to extend if you want to have multiple "containers" like various pockets, backpacks and what not, and manage them separately. This should at least be tried in the context of https://github.com/alan-if/alan-i18n/discussions/18.

tajmone commented 3 years ago

On the matter of aggregating attributes in a container when considering limits, a simplistic approach would be to just recursively sum up the weights. But the matter is actually somewhat more complicated. For weight it is natural to think about aggregation, but what about size? For that attribute the largest size should probably considered, not the sum, but it might also depend on the authors intention with the "size" attribute and how that relates to items being in a container.

Indeed, hence my question of this was a design choice.

I'm aware that when it come to count-limits, it's a well-established tradition in IF to see "carry-all" bags which extend the player's carrying capacity — I personally never liked the fact that these force you to constantly move objects from the bad to the inventory, often having to drop and re-pick some, but that's still a very common case.

There is currently no way to describe how a limit should consider container attributes, so intentional or not, I think it might be the only feasible, admittedly not perfect, option. Or do you have a suggestion?

You're right, since attribute-limits are arbitrary, it would be difficult to find a fit-all solution. So I guess that authors will have to come up with tricks to ensure that physics laws and common sense are not violated — via CHECKs, or other mechanisms, like you mentioned (Sets, etc.). We often see that anyway, e.g. in the StdLib there are similar examples.

Weight and size are just two obvious examples, but I guess there could be many others that we're just not thinking of right now. Also, to complicate things, most game Actors are also Containers, so it's hard to reconcile properties of living containers with inanimate ones (like boxes, bottles, etc.), not to mention lakes and rivers, which are often implemented as containers, and the need to decide which containers should be see-through (even when locked, e.g. a sealed bottle).

But I do think that this topic deserves some clarification in the ALAN Manual, because currently it's not covered, and I was surprised to discover how indirect containment was being handled.

Back to the original problem, the fact that you can pick up a 30 Kg bag, but you can load a carried bag more than 20 Kg (10 Kg at the time) is an inconsistency which is potentially damaging to IF puzzles — i.e. it could be exploited to work around correct solutions, or even break a game by overcoming some intricate logic of the puzzle. So consistent behaviour might be desirable here.

Whatever the attribute might stand for, all syntax constructs that can move an object into a container should abide to the same rules, even for nested containers — I'm thinking of locate, and I'm not sure if there are other constructs that might do this. The problem seems that locateing objects into the bag doesn't track the limits of the Hero which contains the bag, but locateing the bag in the hero does. So the question is whether limits should be inherited by nested objects. I think that this principle applies to any unit of measure really, whether it is weight or size — if the bag contents exceed the carrier's limits then its carrier can't handle them (size or weight). But probably this requires some more brainstorming, for not all examples are equal.

Still, at least warning authors about potentially disruptive workaround that could jeopardize carefully crafted puzzles should be well documented and covered by examples (IF is, after all, very Puzzle-Fest oriented).

As I have mentioned before I think the worn-problem could be solved with a set attribute on the hero. This should also be easy to extend if you want to have multiple "containers" like various pockets, backpacks and what not, and manage them separately. This should at least be tried in the context of alan-if/alan-i18n#18.

I'm not quite convinced of this. I had tested various solutions before deciding upon the new system that I finally proposed for the StdLib, including Sets. The thing is that I decided that sticking to the semantic definition of what makes something a worn item eventually led me to opt for a simple attribute-based solution, and enforcing verbs to honour that attributes. All other solutions, like resorting to EVENTs "as functions", invisible entities, etc., all tend to add complexity and be error prone (and hard to track by authors), whereas simple guidelines are easier to stick with.

Also, the new system doesn't apply only to the Hero, but to any actor, whereas the old approach required a separate system and code for NPCs, which made it even more complex. NPCs didn't have their own worn entity to store worn items, which lead me to believe that neither the Hero needs one after all.

But of course, I'm open to experimentation with Sets, and exploring other paths, if these can lead to better results and, most important, are going to make life easier for authors.

thoni56 commented 3 years ago

Weight and size are just two obvious examples, but I guess there could be many others that we're just not thinking of right now. Also, to complicate things, most game Actors are also Containers, so it's hard to reconcile properties of living containers with inanimate ones (like boxes, bottles, etc.), not to mention lakes and rivers, which are often implemented as containers, and the need to decide which containers should be see-through (even when locked, e.g. a sealed bottle).

But I do think that this topic deserves some clarification in the ALAN Manual, because currently it's not covered, and I was surprised to discover how indirect containment was being handled.

Yes, definitely need to be explicitly mentioned in the manual. (https://github.com/alan-if/alan-docs/issues/111)

Back to the original problem, the fact that you can pick up a 30 Kg bag, but you can load a carried bag more than 20 Kg (10 Kg at the time) is an inconsistency which is potentially damaging to IF puzzles — i.e. it could be exploited to work around correct solutions, or even break a game by overcoming some intricate logic of the puzzle. So consistent behaviour might be desirable here.

To be fair, it is consistent but perhaps not expected or natural.

Whatever the attribute might stand for, all syntax constructs that can move an object into a container should abide to the same rules, even for nested containers — I'm thinking of locate, and I'm not sure if there are other constructs that might do this. The problem seems that locateing objects into the bag doesn't track the limits of the Hero which contains the bag, but locateing the bag in the hero does.

Well, yes, because when locating in the bag it is the limits of the bag that is checked.

So the question is whether limits should be inherited by nested objects. I think that this principle applies to any unit of measure really, whether it is weight or size — if the bag contents exceed the carrier's limits then its carrier can't handle them (size or weight). But probably this requires some more brainstorming, for not all examples are equal.

I'm not quite sure what you are suggesting here. Are you proposing that (e.g.) if a container (object), a bag, is in the heros inventory then the limits from the heros inventory should dynamically apply directly to that container?

If so, given a limit weight 10. on the heroes inventory, then you could carry a stone of 10 and still put another stone of 9 in the bag. I'm not sure this is more "logical".

There are two parts to this problem I think:

  1. when placing something in a container that is in a container the limits of the containing container(s) are not considered. (Which could be implemented fairly easy, but would result in the "inconsistency" that I described above.)
  2. how limits should count when you have nested containers (Which is a broader and more complex question of principles that can be applied to all reasonable cases.)

Perhaps the limit construct should be amended with instructions on how to "aggregate" if and when nested containers are present. Something like

<hero....>
    Container
        Limits
            Sum weight 20.
            Max size 5.

Indicating that the summed value of all weight attributes cannot exceed 20 and the maximum of all size attributes can not exceed 5. Not adding Sum or Max would give the current behaviour, not considering nested container contents, being language backwards compatible.

As I have mentioned before I think the worn-problem could be solved with a set attribute on the hero. This should also be easy to extend if you want to have multiple "containers" like various pockets, backpacks and what not, and manage them separately. This should at least be tried in the context of alan-if/alan-i18n#18.

I'm not quite convinced of this. I had tested various solutions before deciding upon the new system that I finally proposed for the StdLib, including Sets. The thing is that I decided that sticking to the semantic definition of what makes something a worn item eventually led me to opt for a simple attribute-based solution, and enforcing verbs to honour that attributes. All other solutions, like resorting to EVENTs "as functions", invisible entities, etc., all tend to add complexity and be error prone (and hard to track by authors), whereas simple guidelines are easier to stick with.

Good. A solution should be as simple as possible. But no simpler ;-)

But I also hear that part of this, any many other "challenges" are the issue with making resuable code, to prevent duplication, but also to make it easy for authors to follow a suggested pattern. So this is again a reason to revisit this.

Also, the new system doesn't apply only to the Hero, but to any actor, whereas the old approach required a separate system and code for NPCs, which made it even more complex. NPCs didn't have their own worn entity to store worn items, which lead me to believe that neither the Hero needs one after all.

I have only climpsed at that code, but I have no issue with that solution as such. Just trying to figure out better inherent support for what "my" users need ;-)

But of course, I'm open to experimentation with Sets, and exploring other paths, if these can lead to better results and, most important, are going to make life easier for authors.

Definitely. So maybe we can do that in the context of the less complex StarterLib.

tajmone commented 3 years ago

Perhaps the limit construct should be amended with instructions on how to "aggregate" if and when nested containers are present. Something like

<hero....>
    Container
        Limits
            Sum weight 20.
            Max size 5.

Indicating that the summed value of all weight attributes cannot exceed 20 and the maximum of all size attributes can not exceed 5. Not adding Sum or Max would give the current behaviour, not considering nested container contents, being language backwards compatible.

Yes, that would be great. I assume that can't be done now, can it? I've never used Limits expect with a single attribute and value, so I'm not actually fully aware of what type of constructs you can use there, and actually though you could use only one attribute at the time.

So, the change here would be that Sum and Max would also apply to nested containers. Would that also considering parent containers, all the way up the hierarchy? i.e. I'm guessing here that these checks are triggered by Locate, so we can assume that, e.g., a put (obj) 'in' (cont) verb would result in Locate obj in cont (where cont might be inside the Hero, an Actor, or a container carried by either, where each container might have its own limits); hence these checks would have to consider all the chain of containers involved (definitely vertically upward), and since the obj itself might be a container with its own contents, its overall weight/size would have to calculated by inspecting its contents too (vertically downward the hierarchy).

I wonder if things could get a bit messy here, e.g. due to classes not sharing the same attributes being evalued here — e.g. some containers might not have weight limits set, and some class instance not hold such an attribute at all. Whereas VERBs have strict rules that prevent at compile time acting on class instances that are not granted to hold attributes mentioned in the VERB body, how would Limits be able to enforce such strictness (if indeed, it needs to)?

Sounds like a huge leap forward in terms of language changes, even though not a backward breaking change. Obviously it would be a nice change, since it would simplify controlling the behavior of Limits without having to resort to funky code, but like all big changes it also opens the doors to many unknown factors (or, we could say, "unthought of factors", since it's hard to predict all the creative ways IF authors manage to come up with).

thoni56 commented 3 years ago

\

Yes, that would be great. I assume that can't be done now, can it? I've never used Limits expect with a single attribute and value, so I'm not actually fully aware of what type of constructs you can use there, and actually though you could use only one attribute at the time.

You can have multiple limiting attributes, but of course not the aggregates (https://alan-if.github.io/alan-docs/manual-beta/manual.html#_limits).

This was just an idea in the moment, but let's continue thinking about it.

So, the change here would be that Sum and Max would also apply to nested containers. Would that also considering parent containers, all the way up the hierarchy? i.e. I'm guessing here that these checks are triggered by Locate, so we can assume that, e.g., a put (obj) 'in' (cont) verb would result in Locate obj in cont (where cont might be inside the Hero, an Actor, or a container carried by either, where each container might have its own limits); hence these checks would have to consider all the chain of containers involved (definitely vertically upward), and since the obj itself might be a container with its own contents, its overall weight/size would have to calculated by inspecting its contents too (vertically downward the hierarchy).

I wonder if things could get a bit messy here, e.g. due to classes not sharing the same attributes being evalued here — e.g. some containers might not have weight limits set, and some class instance not hold such an attribute at all. Whereas VERBs have strict rules that prevent at compile time acting on class instances that are not granted to hold attributes mentioned in the VERB body, how would Limits be able to enforce such strictness (if indeed, it needs to)?

A container is already defined to take a class of instances. The attributes used in the limits must be defined on the class. Thus all items that can be located in the container will by definition have those attributes, but items in nested containers, not necessarily so, of course.

I'm thinking, only thinking at this point, that the logic needs to be somehing like this:

  1. When an item is located in a container its limits are checked.
  2. If the limits indicate an aggregation/nested operation that operation is performed on the directly contained items and recursively applied to all recursively contained items. If an item does not belong to a class or subclass of the class taken by the primary container the value of those attribute will be handled using three-valued logic (null), giving e.g. 0 for a SUM and MAX aggregated limit etc.
  3. If the container is contained in another container, the limits of that "outer" container will be tried in the same fashion, until the "top" of the current container tree is reached.

It is a bit complicated, but it seems to be fairly straight forward logic-wise. How easy it would be to implement or document, I don't know.

Sounds like a huge leap forward in terms of language changes, even though not a backward breaking change. Obviously it would be a nice change, since it would simplify controlling the behavior of Limits without having to resort to funky code, but like all big changes it also opens the doors to many unknown factors (or, we could say, "unthought of factors", since it's hard to predict all the creative ways IF authors manage to come up with).

Users of all computer languages and tools are notouriously known for inventing ways to use it that the author did not intend or even imagine. But that is also the charm of designing tools, that the actual needs of (some) users might trigger new thinking and evolve the language/tool. ;-)

tajmone commented 3 years ago

Mhh,,, as I was trying to describe this problem in the Manual, while writing about I kept thinking of the initial test that lead to this Issue. I still don't understand why in that transcript the Hero can pick up the bag containing items with total weight 30, but is not able to place three items weighing 10 in the bag that's being carried...

If the locate instruction only checks the weight of the object being located, in relation to the target container's limits, why the third item can't be put in the bag? after all, the bag's weight doesn't change only because it contains items of weight, so in theory during the third transferal operation the Hero should be carrying zero weight or 10 (the latter in case the 'put in' verb first locates the object into the Hero, and then locates it to the bag (due to implicit taking).

This what makes the real code example inconsistent to me: being able to pick up the bag with 3x10 objects/weight, but not being able to put the same three objects in the carried bag, one at the time. If locate only looks at the weight of the object being inserted, both actions should be doable.

This is still puzzling me, so I wasn't able to come up with a text to explain this consistently.

thoni56 commented 3 years ago

I'm so sorry for the confusion. My only excuse is that changes has happened gradually over decades, and my memory might sometimes be from another period in time.

I hade to resort to Alans own test cases and the source, and it shows that any limit is actually interpreted and evaluated recursively (adding to the top containers weight e.g.). This is also the case for the special pseudo-attribute count.

Current rules

To sum up:

There is still the concern that all limting attributes should not be handled this way (and there is no way to override it), and also that count is even more suspicious.

But on the upside, I am now 100% sure that those are the current rules ;-)

Parent containers

What's not handled are limits from parent containers. If you are carrying a bag and put the stone in the bag, then only the limits on the bag is considered and not the limits on the inventory (containing the bag).

And here the logic I proposed earlier could be applied.

tajmone commented 3 years ago

What's not handled are limits from parent containers. If you are carrying a bag and put the stone in the bag, then only the limits on the bag is considered and not the limits on the inventory (containing the bag).

OK, that makes more sense — basically when the hero picks the bag carrying 30Kg there are no checks for the bag contents, for the these checks are carried out on the HERO container (i.e. recursively evaluates all his belongings and their contents). So, I guess that in this case the TAKE verb should "manually check" if the object being picked up is a container, and if so calculate it's overall weight.

As far as the StartLib is concerned, we'll have to do some actual testing with current weight limits. Also, whether a 'put/throw/etc. in' verb first tries implicit takin an object could make a huge difference — without implicit taking, the Hero could throw an object that exceeds the max carried weight by the Hero, which is contradictory (even if the target container can handle it) since it should not possible to lift that weight.

  • if the limit is count then all items recursively are counted

that makes sense, after all what would happen if the hero has a limit of COUNT 1, then picks a bag containing 3 items and then tries to take an extra item from the bag — since limits won't allow it, where would that object end up? remain in the bag? This could potentially lead to stale mechanics in some cases, if for some reason the Hero couldn't drop the bag, since he wouldn't be able to get hold of its contents if all throwing and dropping verbs execute an implicit taking first!

thoni56 commented 3 years ago

I've done some deeper experimentation with nested containers and I think I'm seeing the behaviour that triggered this issue. It looks like a bug but I haven't really figured it out yet. It looks like limits are checked at the wrong container in some cases...

tajmone commented 3 years ago

It looks like a bug but I haven't really figured it out yet. It looks like limits are checked at the wrong container in some cases..

Creating Ad Hoc Testing

I should probably create further ad hoc testing for this, in the Testbed repo, pushing further the limits of Limits (pun intended, do I get a bonus?) — I'm very good at breaking things. I think that a key element for these tests is providing various verbs that test moving around objects (w/ and without Container property and contents) via Verbs, Scripts and Events, with and without implicit taking, and testing for container that have and haven not Limits in the nesting chain.

The hard part is finding a way to make these tests semantically friendly to follow (i.e. naming things in a way that makes it easy to track where the problem might be at any time).

thoni56 commented 3 years ago

I should probably create further ad hoc testing for this, in the Testbed repo, pushing further the limits of Limits (pun intended, do I get a bonus?) — I'm very good at breaking things.

Pun is ok, you get a bonus for breaking things ;-)

The hard part is finding a way to make these tests semantically friendly to follow (i.e. naming things in a way that makes it easy to track where the problem might be at any time).

As the saying goes, the hard part of programming is naming things. And good names really make a big difference. Have you seen Naming as a Process?

On a slightly different positive note, I think I've found the cause of the confusion. In some cases the added item is not recognized as a container and thus not aggregated. The container being added to is always recursively aggregated as per my earlier description, but not always the added item.

One might consider this a bug. And as you mentioned earlier it might make it possible to circumvent some puzzles or even whole games. My only concern is that it might break existing games, or libraries. My vote would go to fixing this anyway as it seems to be in line with the rest of the container logic that now exists. (Clear release notes would point this out of course.) @tajmone , you get to vote too ;-)

If so then this is what needs to be done:

If these are implemented, there is no need for an author to handle locate into a container manually at all.

(We have the concern that SUM isn't the only aggregation that could be valid, but considering that it is the way it is implemented now, I think we should ignore that issue for a time when we have better use cases that requires it.)

A separate issue is if a locate into a container also should ensure that limits to all containing containers are not broken. Currently I feel that this should be the case, and am leaning towards implementing this too. (But as a separate issue.)

I think that fleshing out the description of limits in the manual will be much easier of all these three are done. Then the consistency you were looking for might possibly be there.

tajmone commented 3 years ago

Have you seen Naming as a Process?

I didn't, and thanks for the link! it seems a very good read, and I like to collect links to good articles like this one. Of course, the topic of good naming practices is found in many programming books, but usually nothing more than an ice-breaker chapter, whereas this articles-series seems to elevate the topic to whole other level — and its language agnostic too.

Beta Breaking Changes Justified as Fulfilments of v1 Promises

One might consider this a bug. And as you mentioned earlier it might make it possible to circumvent some puzzles or even whole games. My only concern is that it might break existing games, or libraries. My vote would go to fixing this anyway as it seems to be in line with the rest of the container logic that now exists. (Clear release notes would point this out of course.)

I agree on making this right. Any feature that is not working as documented, or that is inconsistent with the general design of the language, should IMO be fixed — even it might break some past games — because it's like honouring the contract of language promised features of v3.0.0. So, it should be considered acceptable during the Beta stage.

Breaking Changes and Old Games

Also, I believe that for any adventure that strongly relied on the details of Limits mechanics for its puzzles integrity and game success, its author would necessary have come up with a test suite of sorts to tackle its puzzles from every angle and ensure the player can't possibly end in an unwinnable situation. Such a test suite would allow to immediately catch if and how such changes affect the original adventure, making it easy to fix them. If not, then probably the adventure was build without full knowledge and appreciation of these mechanics and their limitations, in which case it shouldn't be an issue either — I mean, it only took me less than an hour to realize that something was not quite right with Limits, so I find it hard to believe that these inconsistencies could go by unnoticed in a game where puzzles revolving around Container Limits are central.

Breaking Changes and Libraries

As for the libraries, my understanding is that they just offer some basic default settings for Count and attribute Limits, but there's no elaborate layer of control built around them — that job is left to the authors. So we should be good with this in terms of libraries. In any case, we'd be updating the StarLibs accordingly (and possibly provide some default control layer too) and as far as the StdLib goes this is the type of change for which Anssi has given me freedom of intervention, as long as I keep him informed about these changes so that he can check them before they're merged into the next release.

If so then this is what needs to be done:

  • [ ] Ensure recursive "aggregation" of attributes/counts for the added item (if a container) is always done when checking the limit
  • [ ] Ensure that recursive aggregation of attributes works even if somewhere in the current tree of nested containers exists instances that don't have the attribute

Both point seem must-be-done to me. As for the latter point, how would that happen? would it imply that once a Limit is defined for a given class, all other classes would implicitly have a 0 valued immutable attribute attached to them? or would it be a simple on-the-fly check for the absence of such attribute?

Exposure of Limits Pseudo-Attributes

I think that an important consideration here would be: how are these pseudo-attributes (Limits and Count) exposed to the language? Are they (or will they be) accessible to the code, e.g.

If Count of Limits of WineJar < [some value]
  then ...

or

If hero:Limits:weight < this:weight
  then ...

Making Limits accessible to author (either as read only, or as writeable attributes) would be a strong improvements in terms of being able to control different use cases of Limits (i.e. volume vs weight, strange cases, etc.), so that VERBs, SCRIPTs and EVENTs could carry out some checks before attempting an actual locate.

Some of the current pseudo attributes are exposed (e.g. visits of here) in some form or another (read only vs writeable). So, lacking an on-error mechanism to catch instructions that failed due to internal checks (Limits, Entered, Exit Check, and so on), the best way to keep track of each step in a VERB/SCRIPT/EVENT is doing is to carry out some preliminary evaluations.

Also, if the values of Limits could be changed mid-game, this would open the door to interesting possibilities and problems workarounds (although I have no clue how any of this practically translates in terms of the current implementation). E.g. a fatigued Hero could experience a drop in his carrying capacity (Set Hero:Limits:weight to 10), complex containers might have a Limit that changes based on the value of two other Limits, etc.

thoni56 commented 3 years ago

Beta Breaking Changes Justified as Fulfilments of v1 Promises

One might consider this a bug. And as you mentioned earlier it might make it possible to circumvent some puzzles or even whole games. My only concern is that it might break existing games, or libraries. My vote would go to fixing this anyway as it seems to be in line with the rest of the container logic that now exists. (Clear release notes would point this out of course.)

I agree on making this right. Any feature that is not working as documented, or that is inconsistent with the general design of the language, should IMO be fixed — even it might break some past games — because it's like honouring the contract of language promised features of v3.0.0. So, it should be considered acceptable during the Beta stage.

Good. Agree. I'll fix that then.

Exposure of Limits Pseudo-Attributes

I think that an important consideration here would be: how are these pseudo-attributes (Limits and Count) exposed to the language? Are they (or will they be) accessible to the code, e.g.

If Count of Limits of WineJar < [some value]
  then ...

or

If hero:Limits:weight < this:weight
  then ...

Making Limits accessible to author (either as read only, or as writeable attributes) would be a strong improvements in terms of being able to control different use cases of Limits (i.e. volume vs weight, strange cases, etc.), so that VERBs, SCRIPTs and EVENTs could carry out some checks before attempting an actual locate.

Some of the current pseudo attributes are exposed (e.g. visits of here) in some form or another (read only vs writeable). So, lacking an on-error mechanism to catch instructions that failed due to internal checks (Limits, Entered, Exit Check, and so on), the best way to keep track of each step in a VERB/SCRIPT/EVENT is doing is to carry out some preliminary evaluations.

Also, if the values of Limits could be changed mid-game, this would open the door to interesting possibilities and problems workarounds (although I have no clue how any of this practically translates in terms of the current implementation). E.g. a fatigued Hero could experience a drop in his carrying capacity (Set Hero:Limits:weight to 10), complex containers might have a Limit that changes based on the value of two other Limits, etc.

Those are good examples of use cases where changing limits would be a valid need.

Exposing them for read might also be relevant, but as you say, lacking the catching, or perhaps a pre-check feature, for limits is really the root problem here. (Which is one reason for a general mechanism to catch interrupted execution in the interpreter.)

thoni56 commented 3 years ago

Latest build has recursive aggregation of attributes of the added item (the bag in our example). For now it crashes if any of the recursively found items don't have the attribute (next todo).

But I'm a bit torn about the count limit. Currently the code counts items in the container being added to recursively. I'm not sure that's correct or not. If you have a bag with two stones in it in your inventory, do you have 1 or 3 items?

In the name of consistency count should aggregate recursively like an attribute (as if every item has a count-attribute that is 1). But does it really make sense from a game/author perspective? I think it is even more dubious than always summing the attribute.

Any preferences, @tajmone?

tajmone commented 3 years ago

Latest build has recursive aggregation of attributes of the added item (the bag in our example). For now it crashes if any of the recursively found items don't have the attribute (next todo).

Sounds good — I mean, the crashing is not an indication that something was missing in the first place.

But I'm a bit torn about the count limit. Currently the code counts items in the container being added to recursively. I'm not sure that's correct or not. If you have a bag with two stones in it in your inventory, do you have 1 or 3 items?

I guess there's not correct answer here and that i might depend on a game needs. Definitely, the famous "carryall" bag is used as an expedient to bypass the count limits in many games, so I guess that in similar use cases it would be disruptive.

In the name of consistency count should aggregate recursively like an attribute (as if every item has a count-attribute that is 1). But does it really make sense from a game/author perspective? I think it is even more dubious than always summing the attribute.

Probably it would be better to leave it as it, for two reasons:

  1. Doesn't break backward compatibility.
  2. At least there's a non-recursive limits authors can use.

Recursively counting the items in the bag is not hard to implement in a 'take' verb (e.g. on the object being taken) or in a 'put_in' verb (checking both the object being put an the target). Sure, having this feature natively handled would spare quite some code, but it's also true that recursive Count limits are now also be easy to implement using a custom attribute with fixed value of 1 (e.g. Limits item 10 would limit to max 10 items, provided Is item 1. is defined on EVERY OBJECT).

New Recursive Keyword?

Another interesting option would be to have an extra keyword for recursion:

Container
  Limits
    Count Recursive 5.
    weight Recursive 10.

or

Container
  Limits
    Recursive Count 5.
    Recursive weight 10.

Ideally, if the Recursive keyword were to work on all Limits types, we'd have both a non-recursive and recursive version not only for Count but also the attribute. This would definitely satisfy all authors, giving them choice on each individual limitation. My guess is that, whatever the container limitation type (weight, volume, count, etc.), it will always be either recursive or not-recursive — no inbetweens.

thoni56 commented 3 years ago

Latest build has recursive aggregation of attributes of the added item (the bag in our example). For now it crashes if any of the recursively found items don't have the attribute (next todo).

Sounds good — I mean, the crashing is not an indication that something was missing in the first place.

No. It's actually a sign of progress ;-) Much like a failed test in TDD is a step forward.

Probably it would be better to leave it as it, for two reasons:

  1. Doesn't break backward compatibility.
  2. At least there's a non-recursive limits authors can use.

Recursively counting the items in the bag is not hard to implement in a 'take' verb (e.g. on the object being taken) or in a 'put_in' verb (checking both the object being put an the target). Sure, having this feature natively handled would spare quite some code, but it's also true that recursive Count limits are now also be easy to implement using a custom attribute with fixed value of 1 (e.g. Limits item 10 would limit to max 10 items, provided Is item 1. is defined on EVERY OBJECT).

And it also makes more sense to count the actual items in the container. If you had 10 bills in your wallet and can only carry one thing, I'd say that you could carry the wallet, whatever content it has.

New Recursive Keyword?

Another interesting option would be to have an extra keyword for recursion:

Agreed. I'll save that for some future discussion. I think what we have now decided on is consistent, reasonable and good enough until we see actual need for further enhancement of the container limits concept.

tajmone commented 3 years ago

No. It's actually a sign of progress ;-) Much like a failed test in TDD is a step forward.

Exactly, I meant the crashing is NOW an indication instead of NOT, i.e. as in it brought to light an underlying problem that was burred before (my eyesight is degrading very badly, I can't read well on the screen and its making it harder for me to spot typos when I write).

If you had 10 bills in your wallet and can only carry one thing, I'd say that you could carry the wallet, whatever content it has.

That's an interesting point. It's still strange that you wouldn't be able to carry two bills, but not so unusual in the context of IF works. IF players are accustomed to some of the "bent logics" commonly found in IF games (especially in the early works) and their apparent contradictions tend to be ignored for the sake of preserving the narrative illusion.

It's a bit like with novels, where the reader doesn't really question the occasional interference of the narrating voice, or who that whispering voice in the background belongs to — in modern novels, the narrating voice is simply accepted as "a fact of fiction", it's there, end of story; but it hasn't been always like this, in early novels from the late 1800s the narrating voice would introduce itself, justifying its presence and clarifying its goals, because readers were not accustomed to this type of narration.

I still have a vague recollection of how playing my first IF games (during the Infocom and home computers era) felt strange, how the commands and parsing system initially seemed unnatural and counter-intuitive (who am I addressing in my orders? myself? who's telling me "You can see..."? etc.). But then, as I learned my way through the commands interfacing system, all these concerned faded into the background, and I'd start to focus on the actual story, absorbed by it.

And, when I come to think about it, there were plenty of things that make much sense in those games, from the "carryall" bag which allowed to hero to carry more than X items, to other quirks. But then, the carryall has to be understood for what it was in its due context, which usually was a sort of reward for having found it, because some puzzles couldn't be solved until you were able to carry many items. I believe that there were also some technical limitations for which the player couldn't carry more than so many objects without a carryall (arrays indexing limits?) and that the carryall was a trick to double that, so developers were trying to sugar coat limitations and sell them as prices. Those tech limitations no longer apply, but somehow the carryall is still found in modern IF.

All that is to say that often we tend to worry excessively about these logic problems, when we look at them through the "developers' lens", whereas most IF players won't probably even notice certain logic flaws — they are playing those games for the fun of it, we worry for the responsibility of it. I mean, how long have Count limits been around for? a decade? two decades? And how many authors or players have ever noticed these problem before?

All the ALAN libraries since v0.4 (if not earlier) seem to have implemented Count limits, yet the issue never came, neither from library developers nor it users.

Sure, this is one of those things than you don't need it until you need it, and once you've discovered the "blemish" you simply can't avoid seeing it everywhere.

Indeed, consistency is important and desirable, and this feature is worth being implemented and documented straight and clear, but beyond that ... cautious choices right now are probably better than hasty choices dictated by "sake of logic", and keeping in mind that no-one noticed the problem in over a decade should be a good reminder on how low the priority of this really is.

Honestly ... we'd never have discovered it if it wasn't for the the i18n library projects — it went by unnoticed in two years of code-revision and test-suiting the whole StdLib (in both English and Italian), until it was brought to attention recently, while revising library code from 2007.

I think that the new attributes being now recursive, and being able to document their behaviour vs Count being non-recursive, and maybe providing some good examples on how to use them (volume, size) and how to code recursive counting, all of this is already a huge step.

Definitely, having a new Recursive keyword for Limits would be "ice on the cake" but, let's be honest ... all of this has less to do with authors' felt needs (who didn't even notice it) as it's about pleasing our desire for design consistency in design (in the language, in its libraries) and maybe the urge of being always in control of details (WARNING: dark forces creeping into the scene, for The Devil®™ is in the details).

What matters most is documentation coverage, for I believe authors can cope with language quirks, but there's nothing as bad as having to discover undocumented quirks and limitations by trial and error, especially if you've planned out an adventure revolving around puzzles that depend on some expected consistent behaviour which the quirk violates (now, that's really bad).

You often don't realize how odd a feature actually is until you try to document it, as it happened in this case. But, as always, this being an IF oriented language and tool, we have to take into consideration that things are never clear-cut as with other languages, because here semantics and ways of speaking play a greater role than logic and maths.

So I guess this was more than just a bug/inconsistency discovery, it was also a refreshing lesson on the general philosophy of IF, its real vs imagined needs, and its traditions (quirks and all).

thoni56 commented 3 years ago

As I have now added the recursive behaviour of limits, both "down" (summing attribute from recursively contained items when adding) and "up" (looking up the containment tree to see if any of the potentially containing containers have limits that are exceeded), and there are regression tests for those, I consider this issue done and will close it. Any further issues can be raised as new issues.

We'll continue a bit on the documentation branch for this though.

Thanks for bringing this to light. It feels good to know that it is "consistent".

tajmone commented 3 years ago

Great! Thanks a lot.

We'll continue a bit on the documentation branch for this though.

You'll surely be able to explain it better than I could.

I'll update the tests in the Alan Testbed, and then start to look into how the StartLib handles its default Limits, by adding a testsuite for the English library first, and then we might discuss how those limits should be handled in terms of library design (and, e.g. if we need to tweak some verbs accordingly).