Skip to content

Conversation

@kayvank
Copy link
Contributor

@kayvank kayvank commented Mar 21, 2022

remove-hardwired-symbols, have not gotten to the the triggeritems yet. All build and test issues resolved.

@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch 2 times, most recently from 9343cc9 to 6fe48fa Compare March 22, 2022 04:29
@kayvank kayvank marked this pull request as ready for review March 22, 2022 04:31
@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch from 6fe48fa to a8613f5 Compare March 22, 2022 04:57
@Mikolaj
Copy link
Member

Mikolaj commented Mar 22, 2022

Cograts on making this compile. Could you try and remove most uses of toContentSymbol? In a separate commit, if you please. The goal of this exercise was to make the ContentSymbol abstract, so using toContentSymbol is a hack piercing through that abstarction, so should only be used if there's a good reason. The API to used instead of toContentSymbol is in Game.LambdaHack.Content.RuleKind (in which toContentSymbol is legit).

remove-hardwired-symbols-and-triggerItem
@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch from a8613f5 to 5bfaee3 Compare March 22, 2022 16:20
@kayvank kayvank requested review from Mikolaj March 23, 2022 18:12
@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch from df8be36 to 270c733 Compare March 23, 2022 23:17
@Mikolaj
Copy link
Member

Mikolaj commented Mar 24, 2022

The failed CI seem to be doctests. I guess they just need updating.

@Mikolaj
Copy link
Member

Mikolaj commented Mar 24, 2022

README tells how to run doctests. It may be easier than finding it in the CI script.

@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch from 270c733 to 4563085 Compare March 24, 2022 05:39
Issue LambdaHack#152, removing uses of toContentSymbol where possible
@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch from 4563085 to dd995ec Compare March 24, 2022 14:32
descIs :: [TriggerItem] -> Text
descIs [] = "trigger an item"
descIs (t : _) = makePhrase [tiverb t, tiobject t]
descIs _ = makePhrase ["fling", "in-range projectile"]
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather inline the whole makePhrase ["fling", "in-range projectile"] into projectI, because descIs is also used in applyIK so we can't change it freely.

issue 152, remove all hardwired item symbols as well as of TriggerItem.
This commit removes the `flingTs` function before continuing to removing
the trigger item. `flingTs`  added text to `tiverb` and `tiobject`. For now
this commit hardwired these texts in the
`Game.LambdaHack.Client.UI.Content.Input` function `descIs`
@kayvank kayvank force-pushed the 152/remove-hardwired-symbols-and-triggerItem branch from 5138a46 to 9aec886 Compare March 27, 2022 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants