idmillington / undum

A client-side framework for narrative hypertext interactive fiction.
https://idmillington.github.com/undum
MIT License
336 stars 80 forks source link

[bug] setQuality on hidden qualities #25

Closed Oreolek closed 10 years ago

Oreolek commented 10 years ago

From the docs:

It is fine to use setQuality if the quality isn't visible, making this the preferred option for all quality modifications.

Reality (if some quality is missed in undum.game.qualities):

if (qualityBlock.size() <= 0) {
  if (newDisplay === null) return;
  qualityBlock = addQualityBlock(quality).hide().fadeIn(500);
}

Uncaught TypeError: Cannot read property 'hide' of null 

Should be something like:

if (qualityBlock.size() <= 0) {
  if (newDisplay === null) return;
  qualityBlock = addQualityBlock(quality);
  if (qualityBlock === null) return;
  qualityBlock.hide().fadeIn(500);
}
idmillington commented 10 years ago

Are you saying here that, if a quality isn't defined at all (and is therefore invisible, since Undum doesn't know about it), it crashes when you try to make it visible?

I definitely agree the error is bad here, but should it fail silently or throw a more useful error?

Oreolek commented 10 years ago

I thought it should automatically work around it but now I reckon it should throw an error.

Failing silently is obviously not good, as it's an error. But silently declaring the quality and setting it to numeric is a bit too much implicit behaviour for the author. So, the clear error message is a good alternative.

idmillington commented 10 years ago

On reflection I think it shouldn't be an error to setQuality on an undefined quality. I like the presumption that qualities can be used without being declared. Declaration is mostly for the UI. So I've made setQuality work by just setting the quality and not creating a UI for unknown qualities. addQualityBlock, however, is explicitly UI, so I've made that throw an error if it is called with an unknown quality.