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

Added cancel icon and fixed the temporary display #2597

Merged
merged 10 commits into from
Aug 13, 2024

Conversation

Joywin2412
Copy link
Contributor

@Joywin2412 Joywin2412 commented Aug 10, 2024

This fixes issue #2170

My wallet address is 0x5212f1ae94e2016c6cd885751156b7bf1db19289

Copy link

vercel bot commented Aug 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
ancientbeast ✅ Ready (Inspect) Visit Preview Aug 13, 2024 1:25pm

@DreadKnight
Copy link
Member

@Joywin2412 This is meant to show up over the passive icon, not above the unit, so no need for new png file.
Also, there's an empty frame being displayed in the top left side of the game's canvas.

@Joywin2412
Copy link
Contributor Author

Hey @DreadKnight , I think the issue is correctly solved right now. Do check it out. I'll clean the pr in the following commits.

@DreadKnight
Copy link
Member

Hey @DreadKnight , I think the issue is correctly solved right now. Do check it out. I'll clean the pr in the following commits.

@Joywin2412 I've tested a few times, but I just don't see the cancel icon showing up over the passive ability's icon...

@Joywin2412
Copy link
Contributor Author

Hey @DreadKnight , I think the issue is correctly solved right now. Do check it out. I'll clean the pr in the following commits.

@Joywin2412 I've tested a few times, but I just don't see the cancel icon showing up over the passive ability's icon...

It works fine for me on all browsers. Probably a port issue. Do check it now again @DreadKnight

@DreadKnight
Copy link
Member

Hey @DreadKnight , I think the issue is correctly solved right now. Do check it out. I'll clean the pr in the following commits.

@Joywin2412 I've tested a few times, but I just don't see the cancel icon showing up over the passive ability's icon...

It works fine for me on all browsers. Probably a port issue. Do check it now again @DreadKnight

Tested again on Firefox, still no cancel icon. Anyway, "assets/icons/next.svg" displays just fine, so clearly there's no need for a png version of the cancel icon.

If the game ran to begin with, there was no issue with the port number.

Perhaps make a video of it working? I wonder if you've implemented it again somewhere else and I'm not looking in the right spot 🐻

@Joywin2412
Copy link
Contributor Author

@DreadKnight I will share the video link using google drive

@DreadKnight
Copy link
Member

@DreadKnight I will share the video link using google drive

You can drop mp4 vids in the comment section and they'll share just fine btw.

@Joywin2412
Copy link
Contributor Author

Joywin2412 commented Aug 12, 2024

@DreadKnight
Copy link
Member

DreadKnight commented Aug 12, 2024

@DreadKnight https://drive.google.com/file/d/1HQBnEIsfXYOvtPHoaARulDSX8Yra4rr5/view?usp=sharing

@Joywin2412 You're probably doing something rather random once again, not sure sigh
Previous time was rather inspiring and will trigger some new issues with new specs for down the line 🐻

Do this regarding testing and what's actually expected:

  1. Materialize a unit like Swine Thug.
  2. Skip turn until Swine Thug is active.
  3. Press the passive icon's ability to toggle between usable abilities.
  4. Every cycle the cancel icon (svg) should show up over the passive ability's icon, just like that next.svg does

Note that there's a bug #2599 and toggling won't cycle properly for all the units, so careful with that.

@Joywin2412
Copy link
Contributor Author

@DreadKnight , I worked on the issue. See if it is the expected fix for the issue.

@DreadKnight
Copy link
Member

DreadKnight commented Aug 12, 2024

@Joywin2412 Tested, it's almost there. A bug left: if toggling too fast and having the cancel icon go over the next icon, it will cause the next icon to never vanish within the active unit's current turn.

@Joywin2412
Copy link
Contributor Author

Joywin2412 commented Aug 12, 2024

@DreadKnight Can I make the icon disabled while transitioning , this is what happens with skip turn too right?

@DreadKnight
Copy link
Member

@DreadKnight Can I make the icon disabled while transitioning , this is what happens with skip turn too right?

Skip turn is different scenario, as a lot of UI needs to be updated and there's a lot going on, so ideally best approach would be to make sure passive ability icon becomes visible after a while without clocking the UI or causing other issues 🐻

@Joywin2412
Copy link
Contributor Author

@DreadKnight Can I make the icon disabled while transitioning , this is what happens with skip turn too right?

Skip turn is different scenario, as a lot of UI needs to be updated and there's a lot going on, so ideally best approach would be to make sure passive ability icon becomes visible after a while without clocking the UI or causing other issues 🐻

Okay!!

@Joywin2412
Copy link
Contributor Author

@DreadKnight I think the fast toggling issue is fixed.

@DreadKnight
Copy link
Member

@DreadKnight I think the fast toggling issue is fixed.

@Joywin2412 Works fine now 👍🏻 Now only left is to clean up the PR a bit, please revert changes on src/creature.ts and webpack.config.js files, as they're not needed.

@DreadKnight
Copy link
Member

@Joywin2412 There's still an issue left. After materializing an unit, there's an empty frame in the upper left corner.

@Joywin2412
Copy link
Contributor Author

@Joywin2412 There's still an issue left. After materializing an unit, there's an empty frame in the upper left corner.

Ok I will look into it.

@Joywin2412
Copy link
Contributor Author

@Joywin2412 There's still an issue left. After materializing an unit, there's an empty frame in the upper left corner.

I can't see it on my laptop. Can you perhaps share the screenshot of it? I think that issue was coming in previous commits.

@DreadKnight
Copy link
Member

@Joywin2412 There's still an issue left. After materializing an unit, there's an empty frame in the upper left corner.

I can't see it on my laptop. Can you perhaps share the screenshot of it? I think that issue was coming in previous commits.

Can no longer seem to reproduce that, so yeah, probably some old cache from previous commit, oh well. Will merge then and send bounty 🐻

@Joywin2412
Copy link
Contributor Author

@Joywin2412 There's still an issue left. After materializing an unit, there's an empty frame in the upper left corner.

I can't see it on my laptop. Can you perhaps share the screenshot of it? I think that issue was coming in previous commits.

Can no longer seem to reproduce that, so yeah, probably some old cache from previous commit, oh well. Will merge then and send bounty 🐻

Thank you!!

@DreadKnight DreadKnight merged commit 95e2416 into FreezingMoon:master Aug 13, 2024
5 of 6 checks passed
@DreadKnight
Copy link
Member

Well done! Bounty sent -> https://www.mintme.com/explorer/tx/0x9841f2c9ff12373d8d00e7621fcaf77e54661af6cd0447a1d6e3c1f382dbc48e 🪙

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants