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

support custom attributes defined in scenarios #1058

Merged
merged 3 commits into from
Jan 29, 2023
Merged

support custom attributes defined in scenarios #1058

merged 3 commits into from
Jan 29, 2023

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Jan 28, 2023

closes #1034

Demo

scripts/play.sh --scenario data/scenarios/Testing/1034-custom-attributes.yaml

Screenshot from 2023-01-28 01-24-38

@kostmo kostmo marked this pull request as ready for review January 28, 2023 09:27
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

This looks awesome! 💜 💙 💚 💛 🧡 ❤️

where
addFg = maybe id (flip withForeColor . toAttrColor) $ fg ca
addBg = maybe id (flip withBackColor . toAttrColor) $ bg ca
addStyle = maybe id (flip withStyle . sum . map toStyle . toList) $ style ca
Copy link
Member

Choose a reason for hiding this comment

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

@kostmo I like that the scenario data are stored as a custom data type, but I wish the conversion to brick and vty types was in TUI (#1043).

We can do it later, but it seems better soon before brick becomes part of our game logic. 😄

data/scenarios/Testing/1034-custom-attributes.yaml Outdated Show resolved Hide resolved
data/scenarios/Testing/1034-custom-attributes.yaml Outdated Show resolved Hide resolved
import GHC.Generics (Generic)
import Graphics.Vty.Attributes
import Numeric (readHex)
import Swarm.TUI.Attr (worldPrefix)
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @xsebek , I think it would be better to do this in a way that didn't depend on anything in Swarm.TUI.*, partly just out of a sense that conceptually the abstract game should not depend on the specifics of the user interface, and partly for the practical reason that we might want to have different interfaces in the future.

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Jan 29, 2023
@mergify mergify bot merged commit 8d80e9f into main Jan 29, 2023
@mergify mergify bot deleted the custom-attrs branch January 29, 2023 07:29
This was referenced Feb 27, 2023
mergify bot pushed a commit that referenced this pull request Mar 1, 2023
Previously, custom attributes with only a foreground or only a background set would inherit the foreground/background to the left of them.  This is due to mistakenly using `currentAttr` instead of `defAttr` in #1058.

| Version | Screenshot |
| --- | --- |
| Before | ![Screenshot from 2023-02-28 22-38-55](https://user-images.githubusercontent.com/261693/222064852-11527d25-67b4-4f81-9733-d11ea40462cc.png) |
| Fixed | ![Screenshot from 2023-02-28 22-44-20](https://user-images.githubusercontent.com/261693/222064883-07edb0dd-a233-4082-97da-c887efce5799.png) |
mergify bot pushed a commit that referenced this pull request Mar 23, 2023
# Demo
    scripts/play.sh --scenario Challenges/hackman.yaml --autoplay
![image](https://user-images.githubusercontent.com/261693/221466237-84c4137e-f9d5-4723-85cd-a0ec301caa45.png)


Makes use of both #1058 and #1023.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Defining custom attributes in Scenario files
3 participants