Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ch 12: Adapt Syntax Definitions #36

Closed
4 tasks done
tajmone opened this issue Nov 19, 2019 · 5 comments
Closed
4 tasks done

Ch 12: Adapt Syntax Definitions #36

tajmone opened this issue Nov 19, 2019 · 5 comments
Assignees
Labels
🕐 add to changes Contents changes that need to be documented 💀 text problems Text problems, typos or obsolete contents.
Milestone

Comments

@tajmone
Copy link
Owner

tajmone commented Nov 19, 2019

  • Get @tessman approval for these changes.
  • Fix document source.
  • Document the changes in CHANGES.md.
  • Update commented annotation in source file.

$12.2 LoadPicture() and PictureinText()

In §12.2. Pictures edited the syntax definition:

LoadPicture("resourcefile", "picture")
LoadPicture("picture")
PictureinText("file", "pic", width, height, preserve)
PictureinText("picture", width, height, preserve)

to make it consistent with the other syntax definitions:

LoadPicture("<resourcefile>", "<picture>")
LoadPicture("<picture>")

PictureinText("<file>", "<pic>", <width>, <height>[, <preserve>])
PictureinText("<picture>", <width>, <height>[, <preserve>])

NOTE@tessman: confirm if I've got optional parameters within […] are correctly!

$12.3 PlaySound() and PlayMusic()

And, in §12.3. Sound and Music, the the syntax definition:

PlaySound(resourcefile, sample, loop, force)
PlayMusic(resourcefile, song, loop, force)

was edited to make it consistent with the other syntax definitions:

PlaySound(<resourcefile>, <sample>, <loop>, <force>)
PlayMusic(<resourcefile>, <song>, <loop>, <force>)

NOTE@tessman: I have the impression that some of these parameters are optional and should be within […]. This might apply to other syntax definitions in this chapter, so it might be worth giving a quick look to it!

@tajmone tajmone added 💀 text problems Text problems, typos or obsolete contents. 🕐 add to changes Contents changes that need to be documented 🕑 pending approval Issue requires approval by Ken Tessman labels Nov 19, 2019
@tajmone tajmone added this to the changes list milestone Nov 19, 2019
@tajmone tajmone changed the title §12.2: Adapt Syntax Definition Ch 12: Adapt Syntax Definitions Nov 19, 2019
tajmone added a commit that referenced this issue Nov 19, 2019
In "12. Resources":
    * Check and fix indentation of all Hugo code.
    * Convert all syntax definitions to literal blocks (See #35).
    * Fix some syntax definitions (See #36).
@tessman
Copy link
Collaborator

tessman commented Nov 23, 2019

I wonder whether it should be or "" in all cases, for consistency. The parameter has to be a value, i.e., a variable or dictionary word, etc.

@tajmone
Copy link
Owner Author

tajmone commented Nov 23, 2019

How would the syntaxes become then? I'm not quite sure where the empty "" should go. Like this:

LoadPicture("<resourcefile>", "<picture>")
LoadPicture("", "<picture>")

PictureinText("<file>", "<pic>", <width>, <height>[, <preserve>])
PictureinText("", <picture>", <width>, <height>[, <preserve>])

@tajmone tajmone mentioned this issue Dec 22, 2019
38 tasks
@tessman
Copy link
Collaborator

tessman commented Dec 31, 2019

Actually, you can LoadPicture("name") without specifying the resource file, in which case it will use the last-used resourcefile.

In terms of optional arguments:

PlayMusic is PlayMusic(resourcefile, songname, volume[, loop[, force]]), and
PlaySound is PlaySound(resourcefile, soundname, volume[, loop [, force]])

(both from the library).

Actually, looking at the source code, volume is also an optional argument, resulting in regular (full) volume playback.

tajmone added a commit that referenced this issue Dec 31, 2019
In "§12.3. Sound and Music", amend the syntax definitions of `PlaySound`
and `PlayMusic` (see #36):

* Add missing `<volume>` parameter.
* Amend parameters optionality.
@tajmone
Copy link
Owner Author

tajmone commented Dec 31, 2019

Ah! So the <volume> parameter was missing altogether from PlaySound and PlayMusic!

So, I've updated only the syntax definition of PlaySound and PlayMusic in §12.3. Sound and Music, which now are:

PlaySound(<resourcefile>, <sample>[, <volume>[, <loop>[, <force>]]])
PlayMusic(<resourcefile>, <song>[, <volume>[, <loop>[, <force>]]])

I've also peeked at resource.h from the HugoLib, and now the definition seems correct (it includes all parameters and covers correctly their optionality).

Probably the missing parameter was due to an older library version being used at the time the chapter was written.

Actually, you can LoadPicture("name") without specifying the resource file, in which case it will use the last-used resourcefile.

So, if I've understood correctly, the Picture* syntaxes were OK as they were after the first edit.

I've also updated CHANGES.md accordingly.

If you can confirm that everything is fine I can close the issue and mark the changes as approved.

@tajmone tajmone closed this as completed Jan 8, 2020
@tajmone tajmone removed the 🕑 pending approval Issue requires approval by Ken Tessman label Jan 9, 2020
@tessman
Copy link
Collaborator

tessman commented Jan 29, 2020

I think that's right, yes. (And also the part about this probably being based on an earlier library version. That would require some more archaeology to determine, but it's a good guess.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕐 add to changes Contents changes that need to be documented 💀 text problems Text problems, typos or obsolete contents.
Projects
None yet
Development

No branches or pull requests

2 participants