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

feat(UI): character preview in traits tab on character creation #5031

Merged
merged 12 commits into from
Jul 22, 2024

Conversation

4nonch
Copy link
Contributor

@4nonch 4nonch commented Jul 21, 2024

Checklist

Required

Optional

  • This is a PR that modifies build process or code organization.
    • Please document the changes in the appropriate location in the doc/ folder.
    • If documentation for this feature or process does not exist, please write it.
    • If the change alters versions of software required to build or work with the game, please document it.

Purpose of change

Character creation menu offers no way to see how your character will look in game with enabled tileset. There could be many traits that influence on character appearance. With mods there could be dozen of different hair/skin/eyes and other types of traits. And you pick that stuff only by guessing. If you don't like the result - you'll have to restart the game and try your luck again or manually edit traits ingame via debugger menu.

I'm offering to make character preview feature that will help players to immediately track character appearance changes and pick the traits they find proper.

Describe the solution

  • "cata_tiles::tile_type_search" method were created from "cata_tiles::draw_from_id_string" for possible reusability later and reduce cata_tiles::draw_from_id_string complexity (actually thought i'll need that part of code to get tile but there were no big need at the end. I'll revert the change if it need to)
  • "cata_tiles::draw_from_id_string" signature changed. "bool as_independent_entity = false" added to specify the need of rendering single tile without considering game map context and player position on a screen
  • "cata_tiles::draw_entity_with_overlays" signature changed. "bool as_independent_entity = false" added to bypass it to "cata_tiles::draw_from_id_string"
  • "void cata_tiles::display_character( const Character &ch, const point &p )" public method were made to display character tile on some point of the screen
  • "character_preview_window" structure were created to handle character window preview
  • "termx_to_pixel_value" and "termy_to_pixel_value" were added to get pixels value of single terminal value (like that 1 termy unit is 8 pixels length)
  • Changes in "avatar::create" and "tab_direction set_traits" in newcharacter.cpp to integrate solution

Describe alternatives you've considered

The character preview window is probably too big. I thought to add additional information to it to make it feel not so empty or to change the design but it requires some conversation.

Testing

Was able to build on windows 10 under MSYS (cmake) with help of build guide
Went through some manual tests in character creation menu and in game. Not noticed any bugs (for now).

Additional context

See a video with a showcase below:
Showcase

@github-actions github-actions bot added the src changes related to source code. label Jul 21, 2024
Copy link
Contributor

autofix-ci bot commented Jul 21, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

  • functionality looks good.
  • could also include it in debug mutation window in later PRs.
  • code (tbf, entirity of rendering functions) could use a bit of refactoring in later PRs.
  • windows build is failing for unknown reasons, will have to pass all tests before merging.

src/newcharacter.cpp Show resolved Hide resolved
src/newcharacter.h Show resolved Hide resolved
src/cata_tiles.cpp Outdated Show resolved Hide resolved
@scarf005 scarf005 requested review from joveeater and olanti-p July 21, 2024 15:26
@scarf005
Copy link
Member

scarf005 commented Jul 21, 2024

/**
* Set to true when running in test mode (e.g. unit tests, checking mods).
* Does not correspond to any game option, but still requires
* caching due to heavy usage.
*/
extern bool test_mode;

clang-tidy has lint issues, and windows test fails. on discord it was suggested that tile_ptr null ptr reference was the cause: there's variable named test_mode to determine whether it's running on test mode. we could use that.

@scarf005 scarf005 self-requested a review July 22, 2024 11:28
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

LGTM

@scarf005 scarf005 changed the title feat(UI): character preview feature on traits tab during character creation feat(UI): preview character on traits tab during character creation Jul 22, 2024
@scarf005 scarf005 changed the title feat(UI): preview character on traits tab during character creation feat(UI): character preview on character creation traits tab Jul 22, 2024
@scarf005 scarf005 changed the title feat(UI): character preview on character creation traits tab feat(UI): character preview in traits tab on character creation Jul 22, 2024
@scarf005 scarf005 merged commit f5a0677 into cataclysmbnteam:main Jul 22, 2024
8 checks passed
@chaosvolt
Copy link
Member

chaosvolt commented Jul 23, 2024

Hmm. I didn't really eyeball this until just now. I feel like it's a bit silly that the preview doesn't show any of the character's items on them, and a giant screen that takes up half the game window to barely use that space for a single sprite is both really not optimal, and at bare minimum is something that really should have an option to disable.

I can also 100% see potential for complaints about not being able to disable this given people might be playing this game in situations where a naked paper doll unavoidably taking up about a quarter of the game window is going to be harder to explain to one's boss than the character sprite would be during normal gameplay...

@4nonch
Copy link
Contributor Author

4nonch commented Jul 23, 2024

preview doesn't show any of the character's items on them

Well, character preview is intended to display changing character appearance based of traits. With clothes it gets less obvious that character changed somehow because it hides the skin. If you insist - i'll try to add a feature to toggle clothes on hotkey

giant screen

Tried to make it scalable. Also you can scale character tile itself by pressing Z / Shift+Z to pick appropriate one. If i'll make small screen with character in the middle of great nothing it will look even weirder. If you have suggestion how to make it better please share, will try to implement.

should have an option to disable.

Will implement that with "disabled" by default.

PR broke curses

Oops. The idea that curses build incapable of referencing to cata_cursesport is quite strange. Will look what i could do about it.

@scarf005
Copy link
Member

maybe we could set the panel size to 1/4 and put it on rightmost, and enable it by default (but configurable)

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

Successfully merging this pull request may close these issues.

3 participants