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

The big, ugly, pending pull request #11

Closed
wants to merge 240 commits into
base: master
from

Conversation

2 participants
@Mikolaj
Member

Mikolaj commented Jul 22, 2011

Ping! :D

I hope this fits the mood of LambdaHack. At least, the monsters have been preserved, though their behaviour is a bit more varied now. Feel free to pick and choose, of course, I'll just reapply the rejected commits as initial commits of Allure --- the distant SF fork.

Mikolaj added some commits Jan 22, 2011

multiple heroes: help text for hero selection keys
And a remark that the keys listed are the default keys,
in case we allow the user to change them (via config file or UI) later.
multiple heroes: a few tweaks around the type of heroes on a level
We should probably use the same type for monsters, to have a number
with which to indicate a targeted monster. Monsters probably don't need
to carry their numbers in them, unlike heroes, because they don't get
moved to and from any "current monster" state field.
enable the look mode
TODO: some actions should behave differently in look mode, e.g., movement
mutiple heroes: Tab cycles among heroes on a level
TODO: currently all but on hero must remain on level 1. Either hero switching
in look mode or via number keys will solve that.
vty support status update: vty maintainer OK with numpad number keys
I'll prepare a patch to differentiate numpad and top row number keys.
multiple heroes: Tab cycles heroes also in look mode
So it's now possible to move more than one hero to levels > 1.
fix of a bug caused by config file options being case insensitive
TODO: perhaps make the options case sensitive; it costs too much debugging.
change datatype of look mode from a tuple to a record
That's after I myself didn't remember what the components are used for.
Now it's documented in record field names.
code naming convention: "player" is the currently selected hero
This is short form of "player-controlled hero".
Other heroes and all heroes in general are just "heroes".
fix issue #8 on kosmikus' fork (High score table, boolean flags)
Exactly as proposed; minimal changes to get it working again.
fix monsters unable to attack selected hero
I misuderstood how the action monad "abort" propagates through nested actions.
all heroes (not only the player) regenrate at once
That's because we really want hero selection to be a purely UI distinction;
otherwise players will waste time micromanaging cycling among heroes before
the end of the turn.
changing hero selection takes no time now
Via a tmp hack, until the TODOs below playerCommand implemented.
the loot of all heroes on the current level now counts for the winnin…
…g score

Only heroes from the current level are taken into account, or leaving
one hero camping near the exit would be a too powerful tactics.
add '*' and '?' to the list of confimation keys
This is to to let the keys that request (more) information (help screen,
item list for dropping, etc.) toggle display of the obtained information off.
No two keys required ('?' and ' '), just tap '?' once, look, tap again.
fix a bug with case sensitivity of config; harden code
The bug was wiping out savegame on every other save attempt. I've forgotten
to set case sensitivity in some of the places config is created. Affects only
my branch. With the new code it's much harder to make such a bug again.

Mikolaj added some commits Jul 8, 2011

add LambdaHack.scores to the release files; see #7 on kosmikus/Lambda…
…Hack

The releases are not very common, so I'd advocate making the high scores
available, so that the players can relate their scores to anything.
@Mikolaj

This comment has been minimized.

Member

Mikolaj commented Jul 23, 2011

I see there is a lot of whitespace changes. Sorry about that --- I've spent enough time on the code that reformatting was very worthwhile for me. I think I was consistent in the changes and most of them adhere to one of the styles you used.

Major changes:

  • +effectToAction :: Effect.Effect -> Actor -> Actor -> Int -> Action (Bool, String)
    items could now be defined in a config file, because, when used, they now have Effects instead of causing hard-coded actions; Effects not only can be evaluated to actions, but also have descriptions, AI valuations, different action for different player commands, etc.
  • rework of the modularization (datatypes, location of functions, decision what is content and what is engine code): Level vs Dungeon, Geometry vs GeometryRnd, Foo vs FooState vs FooAction, items vs. item kinds vs. effects, tiles, vs. tile kinds (this one only started), Movable vs. MovableKind vs. MovableAdd,
  • +lheroes :: Party, -- ^ all heroes on the level
    +lmonsters :: Party, -- ^ all monsters on the level
    multiple heroes, next actor to move searched for in the Party data structure, which is no longer a list sorted by times; heroes and monsters now very interchangeable, but still a long way to get PvsP or a bot doing AIvsAI and run LambdaHack as a screensaver, which I will surely try doing at some point and which would prove the code is extremely well structured
  • lvlChange :: VDir -> Action ()
    rewritten level changing; now it's possible to browse levels other than the current level of the player; e.g., level where other heroes reside or any levels in the targeting mode
  • -lookAround :: Action a
    +targetFloor :: Action ()
    +targetMonster :: Action ()
    +checkCursor :: Action () -> Action ()
    targeting mode replaces look around command; it changes behaviour of some actions; in particular checkCursor is a wrapper for actions that do not make sense if the browsed level is not the one with the current player character on it; the scursor component of the state records the position of the targeting cursor and related data; each hero and monster has his own target, set by AI for monsters and via targeting mode for heroes
  • +advanceTime :: Actor -> Action ()
    +playerAdvanceTime :: Action ()
    time is now advanced manually inside action definitions
  • +deleteActor :: Actor -> State -> State
    +checkPartyDeath :: Action ()
    checking monster and hero death now unified and refactored out; surviving heroes carry on
  • +-- | Resolves the result of an actor running into another.
    +-- This involves switching positions of the two movables.
    +actorRunActor :: Actor -> Actor -> Action ()
    also, a hero bumping into another selects him, bumping into a wall performs a search
  • also: colours and keypresses configurable, colormaps change during the game, frontends refactored accordingly, GTK frontend optimized (not quite enough), colours of items generalized to flavours (color + color name + (in the future) extra description), default config compiled into the binary, extended AI (e.g., monsters use and/or throw all items), juggling multiple perception (from multiple heroes and/or player-controlled monsters)
@Mikolaj

This comment has been minimized.

Member

Mikolaj commented Aug 2, 2011

About EDSLs for game content, with reflection:

The refactorings of items vs. item kinds vs. effects and the same started for movables and now also started for tiles in the Allure fork, are a first step to have DSLs for building game content. The crucial point is that the game should be able to modify the content and dump the new definitions so that the content can be automatically pre-balanced each time new content is added and also continuously changed in response to player actions. Ideally the game would also be able to load the new definitions without recompiling. Also, ideally the DSLs would be EDSLs. These two minor goals are in conflict, though, because AFAIK Haskell does not have (comfortable) reflection and, anyway, reflection is a dangerously powerful tool.

Edit: make that one DSL with 3 main types, since some effects will be shared, say, flaming sword, lava and fire elemental will share the flame effect primitive (or combinator taking duration, power, etc.).

Mikolaj added some commits Aug 4, 2011

revert a part of "abort some actions with an empty message"
This reverts q part of commit 3cd423e
concerning running, because the message for the last move of running
was being lost.
readd the deprecated syntax for quasiquotes, for 6.12.3 compatibility
Since gtk does not work with most 7.*, compatibility with 6.12.3 makes sense,
even at the cost of incompatibility with 8.*.
messageMoreConfirm :: Bool -> Message -> Action Bool
messageMoreConfirm blackAndWhite msg = do
messageAdd (msg ++ more)
if blackAndWhite then displayBW else display

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

This indicates that it'd be better to have one function

 display :: Bool -> ...

that takes the BW mode, and just pass the argument through.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

implemented in 7f49100

message :: Message -> Action ()
message nm = Action (\ s e p k a st ms -> k st nm ())
messageWipeAndSet :: Message -> Action ()
messageWipeAndSet nm = Action (\ s e p k a st ms -> k st nm ())

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

I think the name is too long.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

implemented in a6e2b80

then h
else abortWith "this command does not work on remote levels"
updateAnyActor :: Actor -> (Movable -> Movable) -> Action ()

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

Why not rename Movable to Actor, and Actor to ActorId?

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

implemented in a72f566 and 22d6553

run dir = do
pl <- gets splayer
targeting <- gets (ctargeting . scursor)
if targeting

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

Think about doing the mode dispatch elsewhere.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Noted as a TODO comment in the code; extended a bit.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Implemented in 1e7daa2. Done in the simplest way. There are better ways, for sure (as the message aggregation proopsed in TODO inside the changed code), but I'd need some more complex messages to evaluate (e.g., missed blows, criticals).

-- perhaps, when a weapon is equipped, just say "you hit" or "you miss"
-- and then "nose dies" or "nose yells in pain".
msg = subjectVerbMObject sm verb tm $
if isJust str then " with " ++ objectItem state single else ""

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

Make combat messages less verbose.

@@ -14,7 +14,7 @@ import FOV.Shadow
import Geometry
import Level
data FovMode = Shadow | Permissive Int | Digital Int
data FovMode = Shadow | Permissive Int | Digital Int | Blind

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

Should this really be an FOV, or a modifier?

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Noted as a TODO comment in the code; extended a bit.

-- Note that this does not guarantee an item from the group to be chosen,
-- as the player can override the choice.
getGroupItem :: [Item] -> -- all objects in question
String -> -- name of the group

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

There should be a datatype for item groups.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Fully agreed. Noted as a TODO comment in the code. Extended a bit as well.

, jeffect :: Effect -- ^ the effect when activated
, jcount :: RollQuad -- ^ created in that quantify
, jfreq :: !Int -- ^ created that often
, jpower :: RollQuad -- ^ created with that power

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

I think jpower should be part of the Effect. It doesn't make sense for all items, and will mean different things.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Noted as a TODO comment in the code; edited and discussed a bit.

-- of properties form MovableKind, the intention is they may be modified
-- temporarily, but will return to the original value over time. E.g., HP.
data Movable = Movable
{ mkind :: !MovableKind, -- ^ kind of the movable; TODO: make this Int

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

Probably not an Int, but ...

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Fixed the TODO and added similar for ItemKind (where it's already an Int, but should probably be a newtype of Int).

{ icount :: Int,
itype :: ItemType,
iletter :: Maybe Char } -- inventory identifier
{ ikind :: !Int,

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

I don't think this should be an Int.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Added the TODO. I think I will fix this together with ActorKind and TileKind at some point. I'm a bit worried about not being able to use IntMap if I make it a newtype instead of Int, but there is a generalization of IntMap, so I may try to use it instead.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 23, 2011

Member

Actually, I've manage to do this without the generalization of IntMap, see #13..

if IM.null ms
then nextMove
else let order = Ord.comparing (mtime . snd)
(i, m) = L.minimumBy order (IM.assocs ms)

This comment has been minimized.

@kosmikus

kosmikus Aug 14, 2011

Contributor

We should replace this structure using a priority search queue/tree.

This comment has been minimized.

@Mikolaj

Mikolaj Aug 16, 2011

Member

Fully agreed. Noted as a TODO comment in the code. With some luck such a structure will be updated to 7.4 before I get to it.

@kosmikus kosmikus closed this in f618512 Aug 14, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment