zero-to-mastery / ZTM-Quest

A fun little 2D top-down rpg where you can explore the world of zero-to-mastery. Be prepared for funny and serious content and of course, Bruno.
https://zero-to-mastery.github.io/ZTM-Quest/
27 stars 94 forks source link

Add interaction for computer in room 1 from #35 #36

Closed r4pt0s closed 4 weeks ago

r4pt0s commented 1 month ago

Created in issue #35 , add a interaction to the computer now. It should open a iframe as a popup and load https://zerotomastery.io/career-paths/ Everything from there should just feel like normal browsing the website.

Make sure to make to popup close-able as well.

AryanK1511 commented 1 month ago

Hello @r4pt0s ! This issue seems super interesting to me. Could you please assign it to me so that I can start working on this?

r4pt0s commented 1 month ago

@AryanK1511 sure thing. Go for it πŸš€πŸƒπŸ½β€β™‚οΈβ€βž‘οΈ

AryanK1511 commented 1 month ago

Hello @r4pt0s, I was experimenting with the iframe implementation, but I discovered that the ZTM Career Paths page cannot be embedded as an iframe because they have security settings in place that block embedding.

When I tested it, the iframe displayed an error stating "zerotomastery.io refused to connect." This happens because ZTM uses security headers (like X-Frame-Options) to prevent the site from being loaded inside an iframe, which is why it doesn't work.

player.onCollide('computer', () => {
    computer.play('on');
    showPopup('https://zerotomastery.io/career-paths/'); 
});

Screenshot 2024-10-02 at 11 11 13β€―PM

On the other hand, websites like Wikipedia, which allow embedding, work perfectly fine.

player.onCollide('computer', () => {
    computer.play('on');
    showPopup('https://en.wikipedia.org'); // Provide the website URL here
});

Screenshot 2024-10-02 at 11 16 23β€―PM

For reference, you can also check this Stack Overflow answer explaining the issue in more detail: https://stackoverflow.com/questions/31944552/iframe-refuses-to-display.

Since we can't embed the ZTM page directly, I propose an alternative: we could display a dialogue box with a link to the ZTM site. Additionally, we could add a "Learn More" button in the dialogue that takes users directly to the link.

Let me know what you think I should do with this!

r4pt0s commented 1 month ago

Good job figuring this out @AryanK1511 You are right, we have forbidden accessing zerotomastery.io via iframes.

Instead of the iframe, we can show the following text as you mentioned in a dialog box.


Zero To Mastery Career Paths

Whether you’re a complete beginner or an
experienced professional, figuring out the 
next step in your career can be overwhelming.

Our curated Career Paths provide you with 
the step-by-step roadmap you need to take you from any level, to getting hired 
and advancing your career.

Pick a Career Path [here](https://zerotomastery.io/career-paths/) and start your journey. 

Thanks for proposing this alternative.

AryanK1511 commented 4 weeks ago

Good job figuring this out @AryanK1511 You are right, we have forbidden accessing zerotomastery.io via iframes.

Instead of the iframe, we can show the following text as you mentioned in a dialog box.

Zero To Mastery Career Paths

Whether you’re a complete beginner or an
experienced professional, figuring out the 
next step in your career can be overwhelming.

Our curated Career Paths provide you with 
the step-by-step roadmap you need to take you from any level, to getting hired 
and advancing your career.

Pick a Career Path [here](https://zerotomastery.io/career-paths/) and start your journey. 

Thanks for proposing this alternative.

Alright! I have sent in a PR that implements all of these changes.

Karine91 commented 4 weeks ago

https://github.com/user-attachments/assets/6c6f7642-31d3-498d-bf16-6eda67f12b50

@r4pt0s looks like a bug - there is no info in popup and player can move after popup showed up

AryanK1511 commented 4 weeks ago

@Karine91 That's weird! As soon as I pulled the latest changes, it stopped working. ill look into it right now

AryanK1511 commented 4 weeks ago

@Karine91 and @r4pt0s I figured out what the problem is. Someone has changed the displayDialogueWithoutCharacter function after my changes. Now, it requires you to also have a player which doesn't make sense since this function is not supposed to have a character passed to it.

The player variable is also not being used anywhere in this function. Do you want me to create another issue to work on this?

export async function displayDialogueWithoutCharacter({
    k,
    player,
    text,
    onDisplayEnd = () => {},
}) {
    const dialogueUI = document.getElementById('textbox-container');
    const dialogue = document.getElementById('dialogue');
    const closeBtn = document.getElementById('close');

    let intervalRef = null;

    dialogueUI.style.display = 'block';

    intervalRef = processDialog(dialogue, text); // Call function without character name

    closeBtn.style.display = 'block';

    function onCloseBtnClick() {
        onDisplayEnd();
        dialogueUI.style.display = 'none';
        dialogue.innerHTML = '';
        clearInterval(intervalRef);
        closeBtn.removeEventListener('click', onCloseBtnClick);
        k.canvas.dispatchEvent(
            new CustomEvent('dialogueClosed', { detail: { k, player, text } })
        );
    }

    closeBtn.addEventListener('click', onCloseBtnClick);

    addEventListener('keypress', (key) => {
        if (key.code === 'Enter') {
            closeBtn.click();
        }
    });

    k.canvas.dispatchEvent(
        new CustomEvent('dialogueDisplayed', { detail: { k, player, text } })
    );
}
r4pt0s commented 4 weeks ago

@AryanK1511 lets take this issue to work on it. Do you want to fix it?

There was another pr which added events to fix the controls. I approved and merged it.

Because hey, we do need Issues for hacktoberfest PRs. πŸ˜‰

Thanks for reporting @Karine91 πŸ™πŸ»

AryanK1511 commented 4 weeks ago

@r4pt0s, yeah sure I can work on this

r4pt0s commented 4 weeks ago

Awesome @AryanK1511 πŸ™πŸ»

AryanK1511 commented 4 weeks ago

Screenshot 2024-10-03 at 5 57 21β€―AM

@r4pt0s You might have to create another issue for this since none of the interactions seem to work now

r4pt0s commented 4 weeks ago

Jep, just saw it @AryanK1511

AryanK1511 commented 4 weeks ago

What do you suggest we do then @r4pt0s ?

r4pt0s commented 4 weeks ago

@AryanK1511 just looked it up. the displayDialogueWithoutCharacter function uses the player as event payload so we will keep it for now. Because it gets passed anyway to the interactions function, we can keep it for now.

I would suggest, you just fix your computer interaction and I take care of the other ones okay?

AryanK1511 commented 4 weeks ago

@r4pt0s. Take a look at #67, It should fix the computer interaction.

r4pt0s commented 4 weeks ago

@AryanK1511 @Karine91 fyi: all fixed now. Thanks again for your report and work πŸŽ‰πŸ”₯

AryanK1511 commented 4 weeks ago

yayyyy!!

Karine91 commented 4 weeks ago

@r4pt0s @AryanK1511 And I am here again to report an issue:) User still moving which makes popup appears again and again

https://github.com/user-attachments/assets/50a1c3c7-8e3f-43f0-9a9a-c5592e49c24f

r4pt0s commented 4 weeks ago

@Karine91 you are fire πŸ”₯πŸ˜‚. You have a awesome tester attitude Thanks.

@AryanK1511 wanna look into this?

KevinBatdorf commented 4 weeks ago

Someone has changed the displayDialogueWithoutCharacter function after my changes. Now, it requires you to also have a player which doesn't make sense since this function is not supposed to have a character passed to it.

This was me. The scene was needed so that the dialog could be accessed (and events fired). I added the player because it was passed with the scene and it allows it to be extended in ways such as having the play face the target, run a specific animation, etc. I suggest not removing this requirement. However, even now it's an optional parameter. But I'd still suggest passing it in.

AryanK1511 commented 4 weeks ago

@r4pt0s @AryanK1511 And I am here again to report an issue:) User still moving which makes popup appears again and again

2-Click.Screen.Recorder-20241003-124707.611.mp4

Gotta respect the grind @Karine91 🀣. @r4pt0s I guess I have no choice but to look into this once again haha.

AryanK1511 commented 4 weeks ago

@r4pt0s, @Karine91 Check out #71