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

Add some new pages to addon docs #459

Merged
merged 7 commits into from
Aug 6, 2024

Conversation

Hexagonl
Copy link
Contributor

@Hexagonl Hexagonl commented Jun 22, 2024

I made a draft PR for now because I'd like for @Hans5958 to double check what I've made so far and ensure I'm on the right path. I'll continue making more pages and put the commits for those in this draft PR, to reduce cluttering the repository's PR history.

I'll also propose some guideline additions to the addon docs wiki in a separate issue/PR, which will be based off of stuff I've encountered while making the 60fps addon doc.

Current added addon docs

  • 60fps
  • Paint costume by default

@Hans5958 Hans5958 added scope: docs Related to the documentation (Scratch Addons Docs) type: enhancement New feature or request labels Jun 23, 2024
@Hans5958
Copy link
Member

Hans5958 commented Jun 23, 2024

Thank you! I guess I have few suggestions.

  1. Instead of "Issue #6860", I prefer "#6860". For that kind of usage I would prefer this instead.

    • The addon might be marked as dangerous to curb projects that require this addon on the Scratch website (#6860).

    Although I think "issue #6860" (with a little I) should be used just so the general users know that it refers to an issue, but I'm not sure if this is really needed.

  2. I think in TurboWarp, it is mentioned as "TurboWarp Addons", with a capital Addons.

  3. I would want to see the usage of the footnotes, especially the Trivia which I think you should cite. It works similary like GitHub. For example, like this1 and this.2

    For example, like this[^some-cite-name] and this.[^1]
    
    [^1]: It doesn't have to be a number, but you can.
    [^some-cite-name]: Hello! This should still appear first even though it is written last on Markdown.
    
  4. I just made changes so title: Higher project framerate mode is not needed on the front matter. You can remove it.

  5. I prefer do "The addon is now enabled" (with "The") instead of "Addon is now enabled" (similar to other such Option) but I guess this works.

Footnotes

  1. Hello! This should still appear first even though it is written last on Markdown.

  2. It doesn't have to be a number, but you can.

@WorldLanguages
Copy link
Member

Nice work!

- The addon's functionality is only enabled whenever the user toggles it on by holding `alt` and clicking the green flag. The addon's functionality is toggled off every time the page is opened/refreshed.
- The addon works in both the project page and the editor.
- By default, the addon (when toggled on) sets the project's framerate to 60 FPS. This value can be changed in the addon's settings to an integer ranging from 31 to 240.
- To prevent abuse and cheating in cloud-based multiplayer games, the addon does not work whenever there are cloud variables in a shared project.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we check whether there are cloud variables or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hexagonl Was this removed (or the statement checked?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hexagonl Was this removed (or the statement checked?)

It hasn't been removed. The statement is definitely incorrect - there are no references to cloud variables in the addon's code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- To prevent abuse and cheating in cloud-based multiplayer games, the addon does not work whenever there are cloud variables in a shared project.

Copy link
Contributor Author

@Hexagonl Hexagonl Aug 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hexagonl Was this removed (or the statement checked?)

I verified it myself and I did remove it in my latest commit Never mind I just saw I mistakenly removed the wrong bullet point

@WorldLanguages
Copy link
Member

@Hans5958 Please review again if you have some time.

I don't think we should be strict here, as long as there's not any false statements being added to the docs, any page is better than no page

Copy link
Member

@Hans5958 Hans5958 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, I'm pretty fine to merge this since the beginning. Problem is that this is still a draft, so it is up to him if he wants this to be merged.

content/addons/paint-by-default.md Outdated Show resolved Hide resolved
@WorldLanguages
Copy link
Member

Before merging see: #459 (comment)

@Hexagonl Hexagonl marked this pull request as ready for review August 4, 2024 18:27
@Hexagonl
Copy link
Contributor Author

Hexagonl commented Aug 4, 2024

I fixed my earlier mistake of the cloud variables in the 60fps addon, made the requested changes, and also added an additional reference in the 60fps addon. I also marked as ready to review

@WorldLanguages WorldLanguages changed the title Add addon docs Add some new pages to addon docs Aug 5, 2024
@Hans5958 Hans5958 merged commit d1072ca into ScratchAddons:master Aug 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: docs Related to the documentation (Scratch Addons Docs) type: enhancement New feature or request
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants