magic cleanup

[o11c]

As I discovered in my previous rewrite attempt, the TMWA magic system is among the worst code (including the only known recurring memory leak) and cannot be converted to a safer object-lifetime form because it uses unions in flex/bison.

Therefore, a high priority task is to get rid of flex/bison in favor of a safer alternative. Fortunately, the magic file is small enough to make a one-shot conversion feasible.

I have not yet written the server code, because I wanted to finalize the data format first, although I do not expect it to be terribly difficult with my (GPL3+) SExpr library.

I am making the following changes to the magic file:

• Convert to S-expression structures instead of crazy formatting
• Put the spell invocations directly into the magic file instead of relying on the build-magic script
• Refactor the hitchhiker's guide secret answers into another file so we can ship a full magic file. This will require some simple script changes.
• There are no comments, instead there are strings in scopes that will be ignored. There are only a few scopes where you can't put an ignored string, and you shouldn't put a comment there anyway.
• Addition of a (DISABLED) command that can be used anywhere a comment can, particularly at toplevel so it can be used to disable spells.
• Change assignment to use a new (SET) command.
• Addition of a (BLOCK) command where previous just parentheses were used, in the body of an (IF) statement
• Remove THEN and ELSE
• Get rid of = and <> for comparison, instead use == and !=
• I ended up finding it natural to use traditonal lisp attacked close-parens instead of distinct like I originally though. A lot of time writing Python code, and rainbow parentheses has probably led to this decision.

I have finished toplevel (SET) (CONST) (PROCEDURE) and (TELEPORT-ANCHOR), and have done enough of (SPELL) to finalize the format. I am willing to finish, but would accept help since this is a relatively simple, but lengthy, task. I have written a basic vim syntax-highlighter, but can't figure out some of the more complex things I'd like to do on my own.

I am basing the spell invocations on the ones on the test server - Frost, please confirm that they are the same as on main server.

I am aware that there is an upcoming change to the magic file to add another exclusion to one of the warp checks, and am not including it. Of course, what it really *should* do is just check the mapflags for whether a warp should be allowed ... this might need to be exposed. I really, *really* want to get rid of the map_nr and map_level commands.

In addition, I would like to do the following:

• Get rid of all obsolete code, for maintainability's sake.
• Get rid of the condition check for #alonzialonzo, except the check for the petal.

Some typical snippets of converted code:

Code: Select all

"Special-purpose globals"
(SET obscure_chance 95)
(SET min_casttime 200)

"Schools of magic"
(CONST MAGIC 340)
(CONST LIFE 341)
(CONST WAR 342)
(CONST TRANSMUTE 343)
(CONST NATURE 344)
(CONST ASTRAL 345)

"Default sfx on caster"
(PROCEDURE default_effect ()
    (sfx caster (+ (- school MAGIC) 2) 0))

(PROCEDURE sfx_generic (target)
    (sfx target SFX_DEFAULT 0))

(PROCEDURE set_var (name mask shift value)
    (set_script_variable caster name
        (|  (&  (script_int caster name)
                (neg (<< mask shift)))
            (<< (& value mask) shift))))

"value:                 How many HP we healed"
"gain:                  how many life magic experience points we can potentially gain"
"heal_xp_value_divisor: 1 for instaheal, 2 for slow heal"
"base_exp_factor:       factor for how many base experience points (max) the player should be allowed to gain"
(PROCEDURE gain_heal_xp (value gain heal_xp_value_divisor base_exp_factor)
    (SET last_heal_xp   (&  (>> (script_int caster SCRIPT_XP)
                                SCRIPT_HEALSPELL_SHIFT)
                            SCRIPT_HEALSPELL_MASK))
    (IF (&& (!= target caster)
            (>  (/ value heal_xp_value_divisor)
                (+ 10 last_heal_xp (random (+ last_heal_xp 1)) (random (+ last_heal_xp 1)))))
        (BLOCK
            (SET heal_xp (+ last_heal_xp gain))
            (IF (> heal_xp SCRIPT_HEALSPELL_MASK)
                (BLOCK
                    (SET heal_xp SCRIPT_HEALSPELL_MASK)
                    (set_var SCRIPT_XP SCRIPT_HEALSPELL_MASK SCRIPT_HEALSPELL_SHIFT heal_xp)))))
    (IF (!= target caster)
        (gain_experience caster (* base_exp_factor (extract_healer_experience target value)) 0 1)))

(SPELL () flying-backpack "#plugh" (PC target)
    (LET level 1)
    (LET school NATURE)
    (=> (MANA 12)
        (CASTTIME 1000)
        (REQUIRE (> (skill caster MAGIC) level))
        (REQUIRE (> (skill caster school) level))
        (OR (REQUIRE (> (skill caster school) 3))
            (COMPONENTS "SilkCocoon"))
        (REQUIRE (< (rdistance (location target) (location caster))
                    (+ 2 (/ spellpower 30))))
        (EFFECT
            (CALL adjust_spellpower school)
            (CALL default_effect)
            (IF (!= caster target)
                (sfx caster 2 0))
            (status_change target SC_FLYING_BACKPACK 0 0 0 0 (+ 5000 (* spellpower 500)))
            (message target "Your backpack is lifted by a mystical force; you no longer feel it pressing on your back.")
            (CALL gain_xp 1)
            (ATEND
                (message target "Your backpack is no longer levitating.")
                (sfx target 2 0)))))

(SPELL () transmute-wood-to-figurine "#parum" (STRING name)
    (LET level 0)
    (LET school TRANSMUTE)
    (=> (MANA 5)
        (CASTTIME 4000)
        (REQUIRE (> (skill caster MAGIC) level))
        (COMPONENTS "RawLog")
        (|  (=> (REQUIRE (== name "boo"))
                (EFFECT
                    (CALL adjust_spellpower school)
                    (CALL default_effect)
                    (CALL create_item "MoubooFigurine" 1 "WarpedLog" 40)
                    (CALL gain_xp 1)))
            (=> (REQUIRE (== name "lurk"))
                (EFFECT
                    (CALL adjust_spellpower school)
                    (CALL default_effect)
                    (CALL create_item "WarpedLog" 1 "WarpedLog", 40)
                    (message caster "You have no idea what a Skrytlurk looks like."))))))

(SPELL () magic-blade "#chiza" ()
    (LET level 0)
    (LET school WAR)
    (=> (MANA 9)
        (CASTTIME 500)
        (REQUIRE (> (skill caster MAGIC) level))
        (|
            (=> (COMPONENTS "SharpKnife")
                (EFFECT
                    (CALL adjust_spellpower WAR)
                    (CALL default_effect)
                    (CALL install_melee_spell (+ 10 (/ spellpower 15)) 1200 30)
                    (CALL gain_xp 1)
                    (ATTRIGGER
                        (CALL melee_damage target 60 (+ 5 (str caster))))))
            (=> (COMPONENTS "Knife")
                (EFFECT
                    (CALL adjust_spellpower WAR)
                    (CALL default_effect)
                    (CALL install_melee_spell (+ 10 (/ spellpower 15)) 1200 30)
                    (CALL gain_xp 1)
                    (ATTRIGGER
                        (CALL melee_damage target 40 (+ 5 (str caster)))))))))

(SPELL (LOCAL) mouboo-smell "#s" ()
    (=>
        (MANA 1)
        (REQUIRE name_of(caster) = "MOUBOOTAUR")
        (EFFECT
            (WAIT 30000)
            (FOREACH PC p (rbox (location caster) 30)
                (message p "You notice a strange smell all around you.")))))

The basic schema is this:
A toplevel is one of: assignment const procedure spell teleport-anchor
An assignment is: (SET var expr)
A const is: (CONST var expr)
A procedure is: (PROCEDURE name (args...) statement...)
A spell is (SPELL (flags...) name "#invocation" maybe-type-arg let... spellbody)
A maybe-type-arg is one of: () (type arg)
A spellbody is one of: implication choice effect
An implication is: (=> prereq... spellbody)
A prereq is one of: require catalysts components mana casttime
A choice is: (| spellbody...)
An effect is: (EFFECT statement... maybe-attrigger maybe-atend)

Note that this implies that every spell has at least one effect (specifically, one per branch of a choice).

[Frost]

As far as I understand your proposal, it sounds excellent.

spells-build/build-magic.sh/magic.conf creates confusion. I really like your suggestion to break out "hidden" things into a separate file. Yes, spells-build is identical on the main and testing servers.

A map flag for "don't allow warp" would indeed be far more elegant than the current situation and would prevent problems.

I think it would benefit the game to open Easter Island to chars who have dark petals, and not just those which finished the quest in May 2010.

[Jenalya]

How exactly do you want to handle the Hitchhiker's quest?

I am aware that there is an upcoming change to the magic file to add another exclusion to one of the warp checks, and am not including it.

There are also some new spells in development by MrGrey. Please check back with him about that.

[o11c]

How exactly do you want to handle the Hitchhiker's quest?

Just change functions/strangerquiz.txt to use a new function call, getsecret(), the values of which will be generated from a table in a different file. (It would be possible to write directly into the global temporary variables if not for the fact that the parameter is (quite sensible) generated from pieces. I guess I could instead add a function that can read a variable by name, but I don't think I should encourage that kind of programming - use arrays instead. If not for the fact that the answers are 2-dimensional, I'd suggest that for Hitchhiker's guide. Maybe I should add some string-manipulation functions ... but let's not overcomplicate things while trying to simplify).

(Also sending a PM with a spelling mistake in one of the answers)

[Nard]

I think it would benefit the game to open Easter Island to chars who have dark petals, and not just those which finished the quest in May 2010.

I think it would benefit the game to open Easter Island to all chars, and not just those which finished the quest in May 2010 or bought a dark petal; provided that they complete a quest. Why not adapt Easter 2010 quest such as it becomes a permanent quest? Replace the human Boss by a Monster boss, add a few zombies somewhere, correct the script so you won't be able to give your petal in place of Dark Crystal ... Or maybe simply use it again for Easter or Halloween 2013.

[o11c]

I have finished the conversion of the magic file. I will push this as a commit set to my fork as soon as I have proper internet.

Besides later play-testing of all the spells, I request that every spell be visually reviewed by at least one person. Besides checking for things I was too dazed to see, this will also make you familiar with the new syntax, as well as possibly stirring feelings of "hey, why isn't this spell active." Also, I would like everybody to look at alonzialonzo, because it is by far the most complicated spell.

Starting by looking all all the PROCEDUREs is probably a good help, I'm fairly confident that I translated all of them correctly.

Some of the code has, in my translation, a really large indent, due to repeated if-else-if-else-if chains. This is by design: bad code should look bad.

It does take a while to get used to prefix notation. It's kind of like beginning python when you've only known the C family "This is crazy, who would ever do that" -> "oh hey, that's actually kind of neat."

I will get started on the code portion as soon as I'm done looking at V0id's patch.

[argul]

maybe simply use it again for Easter or Halloween 2013.

Maybe simply do it.

[tsukinokage]

I'm just curious: Why just embed Lua for magic implementation, and eventually replace eathena scripting by lua too?

[Reid]

I'm just curious: Why just embed Lua for magic implementation, and eventually replace eathena scripting by lua too?

The time to change every eAthena scripts into lua is maybe not worth the result.

[Ablu]

The time to change every eAthena scripts into lua is maybe not worth the result.

You will easily get the time back due to an easier to learn and a LOT more flexible language.

Regards
Ablu