Strictify timers

8 files changed, 160 insertions, 264 deletions

diff --git a/src/common/core.cpp b/src/common/core.cpp
index 300f1cc..ae0e3eb 100644
--- a/src/common/core.cpp
+++ b/src/common/core.cpp
@@ -93,7 +93,11 @@ int main(int argc, char **argv)
 
     while (runflag)
     {
-        do_sendrecv(do_timer(gettick_nocache()));
+        // TODO - if timers take a long time to run, this
+        // may wait too long in sendrecv
+        tick_t now = milli_clock::now();
+        interval_t next = do_timer(now);
+        do_sendrecv(next);
         do_parsepacket();
     }
 }
diff --git a/src/common/mmo.hpp b/src/common/mmo.hpp
index 1bbb28a..f35064b 100644
--- a/src/common/mmo.hpp
+++ b/src/common/mmo.hpp
@@ -3,7 +3,7 @@
 #define MMO_HPP
 
 # include "sanity.hpp"
-
+# include "timer.t.hpp"
 # include "utils.hpp"
 
 constexpr int FIFOSIZE_SERVERLINK = 256 * 1024;
@@ -22,9 +22,9 @@ constexpr SkillID get_enum_max_value(SkillID) { return MAX_SKILL; }
 constexpr int GLOBAL_REG_NUM = 96;
 constexpr int ACCOUNT_REG_NUM = 16;
 constexpr int ACCOUNT_REG2_NUM = 16;
-constexpr int DEFAULT_WALK_SPEED = 150;
-constexpr int MIN_WALK_SPEED = 0;
-constexpr int MAX_WALK_SPEED = 1000;
+constexpr interval_t DEFAULT_WALK_SPEED = std::chrono::milliseconds(150);
+constexpr interval_t MIN_WALK_SPEED = interval_t::zero();
+constexpr interval_t MAX_WALK_SPEED = std::chrono::seconds(1);
 constexpr int MAX_STORAGE = 300;
 constexpr int MAX_PARTY = 12;
 
diff --git a/src/common/nullpo.hpp b/src/common/nullpo.hpp
index 2d8644f..305448f 100644
--- a/src/common/nullpo.hpp
+++ b/src/common/nullpo.hpp
@@ -12,7 +12,7 @@
 
 # ifndef BUG_FREE
 #  define nullpo_retr(ret, t) \
-    if (nullpo_chk(__FILE__, __LINE__, __func__, t)) \
+    if (nullpo_chk(__FILE__, __LINE__, __PRETTY_FUNCTION__, t)) \
         return ret;
 # else // BUG_FREE
 #  define nullpo_retr(ret, t) /*t*/
diff --git a/src/common/socket.cpp b/src/common/socket.cpp
index 50c08a0..ac5a17d 100644
--- a/src/common/socket.cpp
+++ b/src/common/socket.cpp
@@ -332,7 +332,7 @@ void WFIFOSET(int fd, size_t len)
         FPRINTF(stderr, "socket: %d wdata lost !!\n", fd), abort();
 }
 
-void do_sendrecv(uint32_t next)
+void do_sendrecv(interval_t next_ms)
 {
     fd_set rfd = readfds, wfd;
     FD_ZERO(&wfd);
@@ -342,8 +342,12 @@ void do_sendrecv(uint32_t next)
             FD_SET(i, &wfd);
     }
     struct timeval timeout;
-    timeout.tv_sec = next / 1000;
-    timeout.tv_usec = next % 1000 * 1000;
+    {
+        std::chrono::seconds next_s = std::chrono::duration_cast<std::chrono::seconds>(next_ms);
+        std::chrono::microseconds next_us = next_ms - next_s;
+        timeout.tv_sec = next_s.count();
+        timeout.tv_usec = next_us.count();
+    }
     if (select(fd_max, &rfd, &wfd, NULL, &timeout) <= 0)
         return;
     for (int i = 0; i < fd_max; i++)
diff --git a/src/common/socket.hpp b/src/common/socket.hpp
index 48caee5..c5db89c 100644
--- a/src/common/socket.hpp
+++ b/src/common/socket.hpp
@@ -7,6 +7,8 @@
 
 # include <cstdio>
 
+# include "timer.t.hpp"
+
 // Struct declaration
 
 struct socket_data
@@ -69,7 +71,7 @@ void delete_session(int);
 /// Make a the internal queues bigger
 void realloc_fifo(int fd, size_t rfifo_size, size_t wfifo_size);
 /// Update all sockets that can be read/written from the queues
-void do_sendrecv(uint32_t next);
+void do_sendrecv(interval_t next);
 /// Call the parser function for every socket that has read data
 void do_parsepacket(void);
 
diff --git a/src/common/timer.cpp b/src/common/timer.cpp
index 92d284c..219efd9 100644
--- a/src/common/timer.cpp
+++ b/src/common/timer.cpp
@@ -1,270 +1,148 @@
 #include "timer.hpp"
 
+#include <sys/stat.h>
 #include <sys/time.h>
 
+#include <cassert>
 #include <cstring>
 
+#include <queue>
+
 #include "cxxstdio.hpp"
 #include "utils.hpp"
 
 #include "../poison.hpp"
 
-static
-struct TimerData *timer_data;
-static
-uint32_t timer_data_max, timer_data_num;
-static
-timer_id *free_timer_list;
-static
-uint32_t free_timer_list_max, free_timer_list_pos;
+struct TimerData
+{
+    /// When it will be triggered
+    tick_t tick;
+    /// What will be done
+    timer_func func;
+    /// Repeat rate - 0 for oneshot
+    interval_t interval;
+};
+
+struct TimerCompare
+{
+    /// implement "less than"
+    bool operator() (TimerData *l, TimerData *r)
+    {
+        // C++ provides a max-heap, but we want
+        // the smallest tick to be the head (a min-heap).
+        return l->tick > r->tick;
+    }
+};
 
-/// Okay, I think I understand this structure now:
-/// the timer heap is a magic queue that allows inserting timers and then popping them in order
-/// designed to copy only log2(N) entries instead of N
-// timer_heap[0] is the size (greatest index into the heap)
-// timer_heap[1] is the first actual element
-// timer_heap_max increases 256 at a time and never decreases
-static
-uint32_t timer_heap_max = 0;
-/// FIXME: refactor the code to put the size in a separate variable
-//nontrivial because indices get multiplied
 static
-timer_id *timer_heap = NULL;
+std::priority_queue<TimerData *, std::vector<TimerData *>, TimerCompare> timer_heap;
 
 
-static
-uint32_t gettick_cache;
-static
-uint8_t gettick_count = 0;
+tick_t gettick_cache;
 
-uint32_t gettick_nocache(void)
+tick_t milli_clock::now(void) noexcept
 {
     struct timeval tval;
     // BUG: This will cause strange behavior if the system clock is changed!
     // it should be reimplemented in terms of clock_gettime(CLOCK_MONOTONIC, )
     gettimeofday(&tval, NULL);
-    gettick_count = 255;
-    return gettick_cache = tval.tv_sec * 1000 + tval.tv_usec / 1000;
-}
-
-uint32_t gettick(void)
-{
-    if (gettick_count--)
-        return gettick_cache;
-    return gettick_nocache();
+    return gettick_cache = tick_t(std::chrono::seconds(tval.tv_sec)
+            + std::chrono::duration_cast<std::chrono::milliseconds>(
+                std::chrono::microseconds(tval.tv_usec)));
 }
 
 static
-void push_timer_heap(timer_id index)
+void push_timer_heap(TimerData *td)
 {
-    if (timer_heap == NULL || timer_heap[0] + 1 >= timer_heap_max)
-    {
-        timer_heap_max += 256;
-        RECREATE(timer_heap, timer_id, timer_heap_max);
-        memset(timer_heap + (timer_heap_max - 256), 0, sizeof(timer_id) * 256);
-    }
-// timer_heap[0] is the greatest index into the heap, which increases
-    timer_heap[0]++;
-
-    timer_id h = timer_heap[0]-1, i = (h - 1) / 2;
-    while (h)
-    {
-        // avoid wraparound problems, it really means this:
-        //   timer_data[index].tick >= timer_data[timer_heap[i+1]].tick
-        if ( DIFF_TICK(timer_data[index].tick, timer_data[timer_heap[i+1]].tick) >= 0)
-            break;
-        timer_heap[h + 1] = timer_heap[i + 1];
-        h = i;
-        i = (h - 1) / 2;
-    }
-    timer_heap[h + 1] = index;
+    timer_heap.push(td);
 }
 
 static
-timer_id top_timer_heap(void)
+TimerData *top_timer_heap(void)
 {
-    if (!timer_heap || !timer_heap[0])
-        return -1;
-    return timer_heap[1];
+    if (timer_heap.empty())
+        return nullptr;
+    return timer_heap.top();
 }
 
 static
-timer_id pop_timer_heap(void)
+void pop_timer_heap(void)
 {
-    if (!timer_heap || !timer_heap[0])
-        return -1;
-    timer_id ret = timer_heap[1];
-    timer_id last = timer_heap[timer_heap[0]];
-    timer_heap[0]--;
-
-    uint32_t h, k;
-    for (h = 0, k = 2; k < timer_heap[0]; k = k * 2 + 2)
-    {
-        if (DIFF_TICK(timer_data[timer_heap[k + 1]].tick, timer_data[timer_heap[k]].tick) > 0)
-            k--;
-        timer_heap[h + 1] = timer_heap[k + 1], h = k;
-    }
-    if (k == timer_heap[0])
-        timer_heap[h + 1] = timer_heap[k], h = k - 1;
-
-    uint32_t i = (h - 1) / 2;
-    while (h)
-    {
-        if (DIFF_TICK(timer_data[timer_heap[i + 1]].tick, timer_data[last].tick) <= 0)
-            break;
-        timer_heap[h + 1] = timer_heap[i + 1];
-        h = i;
-        i = (h - 1) / 2;
-    }
-    timer_heap[h + 1] = last;
-
-    return ret;
+    timer_heap.pop();
 }
 
-timer_id add_timer(tick_t tick, timer_func func, custom_id_t id, custom_data_t data)
+TimerData *add_timer(tick_t tick, timer_func func)
 {
-    timer_id i;
-
-    if (free_timer_list_pos)
-    {
-        // Retrieve a freed timer id instead of a new one
-        // I think it should be possible to avoid the loop somehow
-        do
-        {
-            i = free_timer_list[--free_timer_list_pos];
-        }
-        while (i >= timer_data_num && free_timer_list_pos > 0);
-    }
-    else
-        i = timer_data_num;
-
-    // I have no idea what this is doing
-    if (i >= timer_data_num)
-        for (i = timer_data_num; i < timer_data_max && timer_data[i].type; i++)
-            ;
-    if (i >= timer_data_num && i >= timer_data_max)
-    {
-        if (timer_data_max == 0)
-        {
-            timer_data_max = 256;
-            CREATE(timer_data, struct TimerData, timer_data_max);
-        }
-        else
-        {
-            timer_data_max += 256;
-            RECREATE(timer_data, struct TimerData, timer_data_max);
-            memset(timer_data + (timer_data_max - 256), 0,
-                    sizeof(struct TimerData) * 256);
-        }
-    }
-    timer_data[i].tick = tick;
-    timer_data[i].func = func;
-    timer_data[i].id = id;
-    timer_data[i].data = data;
-    timer_data[i].type = TIMER_ONCE_AUTODEL;
-    timer_data[i].interval = 1000;
-    push_timer_heap(i);
-    if (i >= timer_data_num)
-        timer_data_num = i + 1;
-    return i;
+    return add_timer_interval(tick, std::move(func), interval_t::zero());
 }
 
-timer_id add_timer_interval(tick_t tick, timer_func func, custom_id_t id,
-                             custom_data_t data, interval_t interval)
+TimerData *add_timer_interval(tick_t tick, timer_func func, interval_t interval)
 {
-    timer_id tid = add_timer(tick, func, id, data);
-    timer_data[tid].type = TIMER_INTERVAL;
-    timer_data[tid].interval = interval;
-    return tid;
+    assert (interval >= interval_t::zero());
+
+    TimerData *td = new TimerData();
+    td->tick = tick;
+    td->func = std::move(func);
+    td->interval = interval;
+    push_timer_heap(td);
+    return td;
 }
 
-void delete_timer(timer_id id, timer_func func)
+static
+void do_nothing(TimerData *, tick_t)
 {
-    if (id == 0 || id >= timer_data_num)
-    {
-        FPRINTF(stderr, "delete_timer error : no such timer %d\n", id);
-        abort();
-    }
-#ifndef delete_timer
-    if (timer_data[id].func != func)
-    {
-        FPRINTF(stderr, "Timer mismatch\n");
-        abort();
-    }
-#endif
-    // "to let them disappear" - is this just in case?
-    timer_data[id].func = NULL;
-    timer_data[id].type = TIMER_ONCE_AUTODEL;
-    timer_data[id].tick -= 60 * 60 * 1000;
 }
 
-tick_t addtick_timer(timer_id tid, interval_t tick)
+void delete_timer(TimerData *td)
 {
-    return timer_data[tid].tick += tick;
-}
+    assert (td != nullptr);
 
-struct TimerData *get_timer(timer_id tid)
-{
-    return &timer_data[tid];
+    td->func = do_nothing;
+    td->interval = interval_t::zero();
 }
 
 interval_t do_timer(tick_t tick)
 {
-    timer_id i;
     /// Number of milliseconds until it calls this again
     // this says to wait 1 sec if all timers get popped
-    interval_t nextmin = 1000;
+    interval_t nextmin = std::chrono::seconds(1);
 
-    while ((i = top_timer_heap()) != (timer_id)-1)
+    while (TimerData *td = top_timer_heap())
     {
         // while the heap is not empty and
-        if (DIFF_TICK(timer_data[i].tick, tick) > 0)
+        if (td->tick > tick)
         {
             /// Return the time until the next timer needs to goes off
-            nextmin = DIFF_TICK(timer_data[i].tick, tick);
+            nextmin = td->tick - tick;
             break;
         }
         pop_timer_heap();
-        if (timer_data[i].func)
-        {
-            if (DIFF_TICK(timer_data[i].tick, tick) < -1000)
-            {
-                // If we are too far past the requested tick, call with the current tick instead to fix reregistering problems
-                timer_data[i].func(i, tick, timer_data[i].id, timer_data[i].data);
-            }
-            else
-            {
-                timer_data[i].func(i, timer_data[i].tick, timer_data[i].id, timer_data[i].data);
-            }
-        }
-        switch (timer_data[i].type)
+
+        // If we are too far past the requested tick, call with the current tick instead to fix reregistering problems
+        if (td->tick + std::chrono::seconds(1) < tick)
+            td->func(td, tick);
+        else
+            td->func(td, td->tick);
+
+        if (td->interval == interval_t::zero())
         {
-            case TIMER_ONCE_AUTODEL:
-                timer_data[i].type = TIMER_NONE;
-                if (free_timer_list_pos >= free_timer_list_max)
-                {
-                    free_timer_list_max += 256;
-                    RECREATE(free_timer_list, uint32_t, free_timer_list_max);
-                    memset(free_timer_list + (free_timer_list_max - 256),
-                            0, 256 * sizeof(uint32_t));
-                }
-                free_timer_list[free_timer_list_pos++] = i;
-                break;
-            case TIMER_INTERVAL:
-                if (DIFF_TICK(timer_data[i].tick, tick) < -1000)
-                {
-                    timer_data[i].tick = tick + timer_data[i].interval;
-                }
-                else
-                {
-                    timer_data[i].tick += timer_data[i].interval;
-                }
-                push_timer_heap(i);
-                break;
+            delete td;
+            continue;
         }
+        if (td->tick + std::chrono::seconds(1) < tick)
+            td->tick = tick + td->interval;
+        else
+            td->tick += td->interval;
+        push_timer_heap(td);
     }
 
-    if (nextmin < 10)
-        nextmin = 10;
-    return nextmin;
+    return std::max(nextmin, std::chrono::milliseconds(10));
+}
+
+tick_t file_modified(const char *name)
+{
+    struct stat buf;
+    if (stat(name, &buf))
+        return tick_t();
+    return tick_t(std::chrono::seconds(buf.st_mtime));
 }
diff --git a/src/common/timer.hpp b/src/common/timer.hpp
index b00d48d..876d519 100644
--- a/src/common/timer.hpp
+++ b/src/common/timer.hpp
@@ -1,66 +1,46 @@
 #ifndef TIMER_HPP
 #define TIMER_HPP
 
+# include "timer.t.hpp"
+
 # include "sanity.hpp"
 
-enum TIMER_TYPE
-{
-    TIMER_NONE,
-    TIMER_ONCE_AUTODEL,
-    TIMER_INTERVAL,
-};
-/// This is needed to produce a signed result when 2 ticks are subtracted
-inline
-int32_t DIFF_TICK(int32_t a, int32_t b)
-{
-    return a - b;
-}
+# include <chrono>
+# include <functional>
 
-// TODO replace with std::chrono::time_point and std::chrono::duration
-typedef uint32_t tick_t;
-typedef uint32_t interval_t;
-typedef uint32_t timer_id;
-// BUG: pointers are stored in here
-typedef int32_t custom_id_t;
-typedef int32_t custom_data_t;
-typedef void(*timer_func)(timer_id, tick_t, custom_id_t, custom_data_t);
+/// (to get additional arguments, use std::bind or a lambda).
+typedef std::function<void (TimerData *, tick_t)> timer_func;
 
-struct TimerData
+// updated automatically when using milli_clock::now()
+// which is done only by core.cpp
+extern tick_t gettick_cache;
+
+inline
+tick_t gettick(void)
 {
-    /// When it will be triggered
-    tick_t tick;
-    /// What will be done
-    timer_func func;
-    /// Arbitrary data. WARNING, callers are stupid and put pointers in here
-    // Should we change to void* or intptr_t ?
-    custom_id_t  id;
-    custom_data_t  data;
-    /// Type of timer - 0 initially
-    enum TIMER_TYPE type;
-    /// Repeat rate
-    interval_t interval;
-};
+    return gettick_cache;
+}
 
-/// Server time, in milliseconds, since the epoch,
-/// but use of 32-bit integers means it wraps every 49 days.
-// The only external caller of this function is the core.c main loop, but that makes sense
-// in fact, it might make more sense if gettick() ALWAYS returned that cached value
-tick_t gettick_nocache(void);
-/// This function is called enough that it's worth caching the result for
-/// the next 255 times
-tick_t gettick(void);
+/// Schedule a one-shot timer at the given tick.
+/// The timer will automatically be freed after it is called
+/// (during a do_timer).
+TimerData *add_timer(tick_t t, timer_func f);
 
-timer_id add_timer(tick_t, timer_func, custom_id_t, custom_data_t);
-timer_id add_timer_interval(tick_t, timer_func, custom_id_t, custom_data_t, interval_t);
-//#define delete_timer(tid, func) delete_timer(tid)
-void delete_timer(timer_id, timer_func);
+/// Schedule a recurring timer initially at the given tick.
+/// The timer will automatically reregister itself, with the same
+/// opaque handle, every interval after the tick.
+/// It will never be freed unless you use delete_timer.
+TimerData *add_timer_interval(tick_t, timer_func, interval_t);
 
-tick_t addtick_timer(timer_id, interval_t);
-struct TimerData *get_timer(timer_id tid);
+/// Cancel the given timer.
+/// This doesn't actually remove it, it just resets the functor.
+/// and waits for the the tick to arrive in do_timer.
+void delete_timer(TimerData *);
 
 /// Do all timers scheduled before tick, and return the number of milliseconds until the next timer happens
 interval_t do_timer(tick_t tick);
 
-
+/// Stat a file, and return its modification time, truncated to seconds.
+tick_t file_modified(const char *name);
 
 #endif // TIMER_HPP
diff --git a/src/common/timer.t.hpp b/src/common/timer.t.hpp
new file mode 100644
index 0000000..67d7450
--- /dev/null
+++ b/src/common/timer.t.hpp
@@ -0,0 +1,28 @@
+#ifndef TIMER_T_HPP
+#define TIMER_T_HPP
+
+# include <chrono>
+
+/// An implementation of the C++ "clock" concept, exposing
+/// durations in milliseconds.
+class milli_clock
+{
+public:
+    typedef std::chrono::milliseconds duration;
+    typedef duration::rep rep;
+    typedef duration::period period;
+    typedef std::chrono::time_point<milli_clock, duration> time_point;
+    static const bool is_steady = true; // assumed - not necessarily true
+
+    static time_point now() noexcept;
+};
+
+/// A point in time.
+typedef milli_clock::time_point tick_t;
+/// The difference between two points in time.
+typedef milli_clock::duration interval_t;
+
+/// Opaque type representing an active timer.
+struct TimerData;
+
+#endif // TIMER_T_HPP