oakes / odoyle-rules

A rules engine for Clojure(Script)
The Unlicense
530 stars 20 forks source link

Weird behavior of dynamic rules #27

Closed ertugrulcetin closed 11 months ago

ertugrulcetin commented 11 months ago

There are two important rules: ::update-player-forward and ::move-player. The issue is that ::update-player-forward must run before ::move-player. When I order the rules in this manner, it doesn't work as expected;

(swap! *session o/add-rule player-forward)
(swap! *session o/add-rule move-player)
(swap! *session o/add-rule zoom-camera)

When I change the rule order, ::update-player-forward runs before ::move-player (the positions of zoom-camera and move-player are swapped).

(swap! *session o/add-rule player-forward)
(swap! *session o/add-rule zoom-camera)
(swap! *session o/add-rule move-player)

Here is my rules (not working as expected):

...SOME OTHER RULES...

(def *session (atom (reduce o/add-rule (o/->session) rules)))

(def player-forward
  (o/->rule
    ::update-player-forward
    {:what '[[::time ::delta _]
             [::player ::forward forward {:then false}]
             [::keys-pressed ::keys-pressed keys-pressed {:then false}]]
     :when (fn [_ {:keys [keys-pressed]}]
             (or (get keys-pressed "KeyW") (get keys-pressed "KeyS")))
     :then (fn [_ {:keys [forward keys-pressed]}]
             (o/insert! ::player ::forward (cond-> forward
                                                   (get keys-pressed "KeyW") inc
                                                   (get keys-pressed "KeyS") dec)))}))

(def move-player
  (o/->rule
    ::move-player
    {:what '[[::time ::delta _]
             [::player ::forward forward {:then false}]
             [::player ::right right {:then false}]
             [::player ::mesh player]
             [::player ::velocity-ref v-ref]
             [::player ::speed speed]
             [::camera ::camera camera]]
     :when (fn [_ {:keys [forward right]}]
             (or (not= 0 forward) (not= 0 right)))
     :then (fn [_ {:keys [camera forward right speed player v-ref]}]
             (let [forward-dir (j/call camera :getDirection api/v3-forward)
                   forward-dir (v3 (* forward (j/get forward-dir :x)) 0 (* forward (j/get forward-dir :z)))
                   right-dir (j/call camera :getDirection api/v3-right)
                   right-dir (v3 (* right (j/get right-dir :x)) 0 (* right (j/get right-dir :z)))
                   x (+ (j/get forward-dir :x) (j/get right-dir :x))
                   z (+ (j/get forward-dir :z) (j/get right-dir :z))
                   {:keys [x z]} (j/lookup (api/normalize (v3 x 0 z)))]
               (api/set-linear-velocity player (v3 (* speed x) (j/get v-ref :y) (* speed z)))
               (o/insert! ::player {::forward 0 ::right 0})))}))

(def zoom-camera
  (o/->rule
    ::zoom-camera-when-collision
    {:what '[[::time ::delta _]
             [::player ::mesh player]
             [::camera ::camera camera]
             [::camera ::ray-cast-result ray-cast-result]]
     :then (fn [_ {:keys [player camera ray-cast-result]}]
             (let [player-pos (api/get-pos player)
                   camera-pos (j/get camera :globalPosition)
                   result (api/raycast-to-ref player-pos camera-pos ray-cast-result)
                   hit? (j/get result :hasHit)]
               (o/insert! ::camera ::ray-hit? hit?)
               (when hit?
                 (j/update! camera :radius - 0.5))))}))

(swap! *session o/add-rule player-forward)
(swap! *session o/add-rule move-player)
(swap! *session o/add-rule zoom-camera)
nivekuil commented 11 months ago

O'doyle doesn't have a concept of salience so you can't rely on rules firing in a certain order regardless of how they're defined. I think to update the same fact from multiple rules you need to be able to swap! values but o'doyle only has upsert-fact which is like reset!. I'm not sure of the right approach myself, but for doing something like taking damage and regening health in the same tick I just have it all in one rule, but that's not possible with dynamic rules.

Unrelated, but I think using o/ruleset is more readable even for single rules.

ertugrulcetin commented 11 months ago

I don't believe this is related to the salience feature; somehow, dynamic rules behave very differently. Using add-rule in a different order shouldn't affect the behavior of the rule engine.

ertugrulcetin commented 11 months ago

@oakes any updates on this?

oakes commented 11 months ago

I don't see how it could be related to dynamic rules; ruleset is ultimately calling ->rule underneath (a different arity, but they pretty much are the same). If you change your code to use separate rulesets for player-forward, move-player, and zoom-camera, I'm guessing you'll see the same issue.

The order that rules run in a given iteration of fire-rules is determined by then-queue and then-finally-queue, which are sets, so the order is effectively random (based on how the values hashed). Those values include the memory node id, which is just an incrementing number, so rules added earlier have lower memory node ids. I'm guessing that by changing the order in which you add the rules, it is affecting how their memory node ids are hashing, which causes the order in which they fire to change. Definitely not something one can rely on :D

It looks like the issue here is that you have two rules that are both modifying the same state and are both triggered mostly the same way (via the ::time ::delta fact). In its current form, I think it's inherently in conflict -- which one is supposed to take precedence? As @nivekuil mentioned, maybe this should be expressed as a single rule? Sometimes, a good ol' fashioned if or cond within a rule is better than separate rules, if order/precedence is important.

Having something like o/insert that behaves like swap! (presumably called o/update) is something I've thought about, but I don't think it'd necessarily fix this. A pair of updates can lead to a non-deterministic results just like a pair of inserts. You have another location to add some conditional logic, but the original problem remains the same -- your rules are contradicting each other, and now you need to decide who wins.

ertugrulcetin commented 11 months ago

@oakes thank you for the long and detailed explanation! I'll see what I can do for my use case.