[Feat] AchievementPopup parity

Open
  • Is something wrong?
#20 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
Half a day
Newbie friendliness
63/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
lua
Domain
frontend

Research direction

Start from the prettyui.AchievementPopup implementation and its show method, then compare its layout with the regular achievement popup and the task-complete window referenced in the sample code. Inspect how itemSlot and spriteID are configured, since the feature needs a sprite-only mode without item-slot inset borders and an optional subtitle field. The default parent should move to id.Component.TOPLEVEL_V2__TASK_COMPLETE_WINDOW; done means the popup matches standard achievement sizing and placement without the manual workaround.

Written by the indexing model from the issue text.

Description

enhancement good first issue

While they're both very similar, the current AchievementPopup is designed more like collection log popups rather than achievement popups:

Things missing are the ability to have a subtitle field (making it a bit taller), and the ability to use a sprite rather than an object/item with inset borders. It would be nice to be able to use things like the default achievement icon, combat mastery icons, or generic skill icons instead of an item.

The current naive workaround for using a sprite is simply to set the associated object to -1 immediately after creating the popup. Though this still has the ItemSlot inset borders and the size is wrong, so it would still need a lot more manual faffing around to get the alignment the same as standard achievements:

local popup = prettyui.AchievementPopup.show({
	title = 'Title',
	text = 'Text',
	item = config.Obj.YELLOW_PARTYHAT,
	parent = ui.Interfaces:GetComponent(id.Component.TOPLEVEL_V2__TASK_COMPLETE_WINDOW),
})
popup.itemSlot.item:SetAssociatedObject(-1)
popup.itemSlot.item.spriteID = config.Achievement.FromID(1287).spriteID

Also, as in that example above, I think it would make a lot more sense to have the default parent be the task complete window rather than the whole game area, so it appears in the same location as regular achievement popups for anyone who has moved it in edit layout mode, rather than appearing top center by default:

Dominant language
Lua
Stars
6
Forks
4
Avg merge
2d 7h
Merged PRs (30d)
18

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PrettyRS/prettyui

All issues in PrettyRS/prettyui

Similar issues

More Lua issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.