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.

Sign up for GitHub

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

Jump to bottom

Fixed outdated code and documentation in A-Frame tutorial in Games section #34159

Merged
merged 11 commits into from
Jul 11, 2024
Merged

Fixed outdated code and documentation in A-Frame tutorial in Games section #34159

merged 11 commits into from
Jul 11, 2024

Conversation

low-perry
Copy link
Contributor

@low-perry low-perry commented Jun 14, 2024 •
edited
Loading

Description

Changed the docs in order for the code presented to work, since A-FRAME has probably deprecated some of the code used. Replaced the jsfiddle iframe with a version that uses the updated code.

Motivation

It makes the reader follow the tutorial and have a finished product at the end that works

Additional details

This docs also contain a link to a github repo with a solution that does not work. I already created a pull request there, to update the code.

Related issues and pull requests

Fixes #34126
@low-perry low-perry requested a review from a team as a code owner June 14, 2024 11:07
@low-perry low-perry requested review from Elchi3 and removed request for a team June 14, 2024 11:07
@github-actions github-actions bot added Content:Games Games docs size/s [PR only] 6-50 LoC changed labels Jun 14, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 14, 2024 •
edited
Loading

Preview URLs

External URLs (4)

URL: /en-US/docs/Games/Techniques/3D_on_the_web/Building_up_a_basic_demo_with_A-Frame
Title: Building up a basic demo with A-Frame

(comment last updated: 2024-07-11 10:00:29)
@low-perry @github-actions
Update files/en-us/games/techniques/3d_on_the_web/building_up_a_basic…
71f382f 
…_demo_with_a-frame/index.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@bsmth bsmth self-requested a review June 14, 2024 11:49
@low-perry @bsmth
Update files/en-us/games/techniques/3d_on_the_web/building_up_a_basic…
b7d40de 
…_demo_with_a-frame/index.md

Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
@bsmth
Copy link
Member

bsmth commented Jun 14, 2024

Thanks for the additional fixes. I'm going to have to come back to this one as there's more to consider. In general, we want to get rid of JSFiddle examples & links to other repos that we don't maintain because they diverge so much over time. Usually for live samples, we can embed and render the output directly in the page, but we need aframe as a dependency, so I'm going to think about alternatives first before merging. Thanks for your patience 🙌🏻
@low-perry
Copy link
Contributor Author

low-perry commented Jun 18, 2024 •
edited
Loading

@bsmth
would it be ok, to just use the the Aframe library from the jsDelivr CDN. E.g (if the final render is on the Conclusion section):

  <head>
  <script src="https://cdn.jsdelivr.net/gh/aframevr/aframe@3f0df33946cf12e3d552b3b4e500dd5b8ff6f692/dist/aframe-master.min.js"></script>
</head>
<body>
    <a-scene>
        <a-sky color="#DDDDDD"></a-sky>
        <a-light type="directional" color="#FFF" intensity="0.5" position="-1 1 2"></a-light>
        <a-light type="ambient" color="#FFF"></a-light>

        <a-camera
            position="0 1 4"
            cursor-visible="true"
            cursor-scale="2"
            cursor-color="#0095DD"
            cursor-opacity="0.5">
        </a-camera>

        <a-box color="#0095DD" position="0 1 0" rotation="20 40 0" animation="property: rotation;from:20 0 0; to: 20 360 0;dir: alternate; loop: true; dur: 4000; easing: easeInOutQuad;"></a-box>
        <a-entity
            geometry="
            primitive: torus;
            radius: 1;
            radiusTubular: 0.1;
            segmentsTubular: 12;"
            material="
            color: #EAEFF2;
            roughness: 0.1;
            metalness: 0.5;"
            rotation="10 0 0"
            position="-3 1 0"
            animation="property: scale; to: 1 0.5 1; dir: alternate; dur: 2000; loop: true; easing: linear;">
        </a-entity>
    </a-scene>
   
    <a-light type="directional" color="#FFF" intensity="0.5" position="-1 1 2">
    </a-light>
    <a-light type="ambient" color="#FFF"></a-light>
    <script>
      const scene = document.querySelector("a-scene");
      const cylinder = document.createElement("a-cylinder");
      cylinder.setAttribute("color", "#FF9500");
      cylinder.setAttribute("height", "2");
      cylinder.setAttribute("radius", "0.75");
      cylinder.setAttribute("position", "3 1 0");
      scene.appendChild(cylinder);
      let t = 0;
      function render() {
          t += 0.01;
          requestAnimationFrame(render);
          cylinder.setAttribute("position", `3 ${Math.sin(t * 2) + 1} 0`);
      }
      render();
  </script> 
</body>

{{ EmbedLiveSample('Active_learning_creating_your_first_HTML_element', 700, 400, "", "") }}

It seems to work fine, but maybe there is something I am missing!
the html can be hidden since we already show throughout the tutorial how the code is supposed to look like.
What do you think?
@bsmth bsmth changed the title Fixed outdated code and documentation Jun 19, 2024
@bsmth
Copy link
Member

bsmth commented Jun 19, 2024

I think we cannot run JavaScript served from jsdelivr on MDN pages due to security concerns, so we'll need an alternative that might take a little time to get right. Hopefully I have some more information about this soon.
@low-perry
Copy link
Contributor Author

low-perry commented Jun 19, 2024 •
edited
Loading

I am sorry, for keeping you here, but do you mean cannot as in it's not allowed by certain guidelines?
How about using a specific version of file? jsDeliver supports versioning, and i think commits are immutable, but not sure.
Also I noticed that most of the game development tutorials have embedded jsFiddle in them, and since we're talking about it, i thought it's worth mentioning.
@bsmth bsmth self-requested a review July 11, 2024 08:49
@bsmth bsmth removed the request for review from Elchi3 July 11, 2024 09:58
@bsmth
Copy link
Member

bsmth commented Jul 11, 2024

Heya @low-perry I've come back to this and I don't think it's worth blocking this PR from going in despite us wanting to move away from JSFiddles. Let's get your improvements landed and we'll tackle the embeds separately at a later stage. Thanks a lot!
@bsmth
Copy link
Member

bsmth commented Jul 11, 2024

I am sorry, for keeping you here, but do you mean cannot as in it's not allowed by certain guidelines? How about using a specific version of file? jsDeliver supports versioning, and i think commits are immutable, but not sure. Also I noticed that most of the game development tutorials have embedded jsFiddle in them, and since we're talking about it, i thought it's worth mentioning.

Yes, and we have CSP rules for resources we explicitly allow and I don't think we'd add jsDeliver to that.

We want to remove jsFiddles in general because they're hard (impossible) to maintain, really.

When it comes to the examples, one good question is whether it's necessary to embed it in the page at all. We have a lot of standalone examples that are good when served by themselves and we link to them, like the source in a repo:
bsmth
bsmth approved these changes Jul 11, 2024
View reviewed changes
Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

Going to leave a +1 on this one! Thank you for your work and your patience!

I'll keep you posted on any updates when we start to move examples if you like!
@bsmth bsmth merged commit 61451c0 into mdn:main Jul 11, 2024
8 checks passed
@low-perry
Copy link
Contributor Author

@bsmth Thank you for your patience. I am very grateful that you took the time to reply to my questions. And I would love to be updated, when a better solution comes along. 🧡
evelinabe pushed a commit to evelinabe/mdn-content that referenced this pull request Jul 12, 2024
@low-perry @github-actions
@bsmth @evelinabe
Fixed outdated code and documentation in A-Frame tutorial in Games se…
d1c5035 
…ction (mdn#34159)

* Fixed outdated code and documentation

* Fixed linting issue.

* Update files/en-us/games/techniques/3d_on_the_web/building_up_a_basic_demo_with_a-frame/index.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Update files/en-us/games/techniques/3d_on_the_web/building_up_a_basic_demo_with_a-frame/index.md

Co-authored-by: Brian Thomas Smith <brian@smith.berlin>

* fixed animation bug on the a-entity element

* Updates jsfiddle to mirror the changes.

* fixed unsafe html

* Update files/en-us/games/techniques/3d_on_the_web/building_up_a_basic_demo_with_a-frame/index.md

Co-authored-by: Brian Thomas Smith <brian@smith.berlin>

* Update files/en-us/games/techniques/3d_on_the_web/building_up_a_basic_demo_with_a-frame/index.md

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Games Games docs size/s [PR only] 6-50 LoC changed
2 participants
@low-perry @bsmth