Refactored the plugin system to use a hook table. This should help to reduce the probability of data races and strange bugs, as-well as helping to abstract certain details, so future changes are less likely to break things.

Fixed the missing background for the create topic non-JS page in Nox.
The non-JS create topic page now has a localised title.

common/email.go

@@ -29,9 +29,9 @@ func SendValidationEmail(username string, email string, token string) bool {
 // TODO: Add support for TLS
 func SendEmail(email string, subject string, msg string) bool {
 	// This hook is useful for plugin_sendmail or for testing tools. Possibly to hook it into some sort of mail server?
-	// TODO: Abstract this
-	if Vhooks["email_send_intercept"] != nil {
-		return Vhooks["email_send_intercept"](email, subject, msg).(bool)
+	ret, hasHook := GetHookTable().VhookNeedHook("email_send_intercept", email, subject, msg)
+	if hasHook {
+		return ret.(bool)
 	}
 	body := "Subject: " + subject + "\n\n" + msg + "\n"
 

common/extend.go

@@ -12,6 +12,7 @@ import (
 	"errors"
 	"log"
 	"net/http"
+	"sync"
 	"sync/atomic"
 
 	"../query_gen/lib"
@@ -48,40 +49,133 @@ var hookTableBox atomic.Value
 // TODO: Make the RunXHook functions methods on HookTable
 // TODO: Have plugins update hooks on a mutex guarded map and create a copy of that map in a serial global goroutine which gets thrown in the atomic.Value
 type HookTable struct {
-	Hooks          map[string][]func(interface{}) interface{}
-	Vhooks         map[string]func(...interface{}) interface{}
-	VhookSkippable map[string]func(...interface{}) (bool, RouteError)
-	Sshooks        map[string][]func(string) string
-	PreRenderHooks map[string][]func(http.ResponseWriter, *http.Request, *User, interface{}) bool
+	Hooks           map[string][]func(interface{}) interface{}
+	Vhooks          map[string]func(...interface{}) interface{}
+	VhookSkippable_ map[string]func(...interface{}) (bool, RouteError)
+	Sshooks         map[string][]func(string) string
+	PreRenderHooks  map[string][]func(http.ResponseWriter, *http.Request, *User, interface{}) bool
 
 	// For future use:
 	messageHooks map[string][]func(Message, PageInt, ...interface{}) interface{}
 }
 
 func init() {
-	hookTableBox.Store(new(HookTable))
+	RebuildHookTable()
+}
+
+// For extend.go use only, access this via GetHookTable() elsewhere
+var hookTable = &HookTable{
+	map[string][]func(interface{}) interface{}{
+		"forums_frow_assign":       nil,
+		"topic_create_frow_assign": nil,
+	},
+	map[string]func(...interface{}) interface{}{
+		"forum_trow_assign":       nil,
+		"topics_topic_row_assign": nil,
+		//"topics_user_row_assign": nil,
+		"topic_reply_row_assign": nil,
+		"create_group_preappend": nil, // What is this? Investigate!
+		"topic_create_pre_loop":  nil,
+	},
+	map[string]func(...interface{}) (bool, RouteError){
+		"simple_forum_check_pre_perms": nil,
+		"forum_check_pre_perms":        nil,
+	},
+	map[string][]func(string) string{
+		"preparse_preassign": nil,
+		"parse_assign":       nil,
+	},
+	nil,
+	nil,
+}
+var hookTableUpdateMutex sync.Mutex
+
+func RebuildHookTable() {
+	hookTableUpdateMutex.Lock()
+	defer hookTableUpdateMutex.Unlock()
+	unsafeRebuildHookTable()
+}
+
+func unsafeRebuildHookTable() {
+	ihookTable := new(HookTable)
+	*ihookTable = *hookTable
+	hookTableBox.Store(ihookTable)
+}
+
+func GetHookTable() *HookTable {
+	return hookTableBox.Load().(*HookTable)
 }
 
 // Hooks with a single argument. Is this redundant? Might be useful for inlining, as variadics aren't inlined? Are closures even inlined to begin with?
-var Hooks = map[string][]func(interface{}) interface{}{
-	"forums_frow_assign":       nil,
-	"topic_create_frow_assign": nil,
+func (table *HookTable) Hook(name string, data interface{}) interface{} {
+	hooks, ok := table.Hooks[name]
+	if ok {
+		for _, hook := range hooks {
+			data = hook(data)
+		}
+	}
+	return data
+}
+
+// To cover the case in routes/topic.go's CreateTopic route, we could probably obsolete this use and replace it
+func (table *HookTable) HookSkippable(name string, data interface{}) (skip bool) {
+	hooks, ok := table.Hooks[name]
+	if ok {
+		for _, hook := range hooks {
+			skip = hook(data).(bool)
+			if skip {
+				break
+			}
+		}
+	}
+	return skip
 }
 
 // Hooks with a variable number of arguments
-var Vhooks = map[string]func(...interface{}) interface{}{
-	"forum_trow_assign":       nil,
-	"topics_topic_row_assign": nil,
-	//"topics_user_row_assign": nil,
-	"topic_reply_row_assign": nil,
-	"create_group_preappend": nil, // What is this? Investigate!
-	"topic_create_pre_loop":  nil,
+// TODO: Use RunHook semantics to allow multiple lined up plugins / modules their turn?
+func (table *HookTable) Vhook(name string, data ...interface{}) interface{} {
+	hook := table.Vhooks[name]
+	if hook != nil {
+		return hook(data...)
+	}
+	return nil
+}
+
+func (table *HookTable) VhookNoRet(name string, data ...interface{}) {
+	hook := table.Vhooks[name]
+	if hook != nil {
+		_ = hook(data...)
+	}
+}
+
+// TODO: Find a better way of doing this
+func (table *HookTable) VhookNeedHook(name string, data ...interface{}) (ret interface{}, hasHook bool) {
+	hook := table.Vhooks[name]
+	if hook != nil {
+		return hook(data...), true
+	}
+	return nil, false
 }
 
 // Hooks with a variable number of arguments and return values for skipping the parent function and propagating an error upwards
-var VhookSkippable = map[string]func(...interface{}) (bool, RouteError){
-	"simple_forum_check_pre_perms": nil,
-	"forum_check_pre_perms":        nil,
+func (table *HookTable) VhookSkippable(name string, data ...interface{}) (bool, RouteError) {
+	hook := table.VhookSkippable_[name]
+	if hook != nil {
+		return hook(data...)
+	}
+	return false, nil
+}
+
+// Hooks which take in and spit out a string. This is usually used for parser components
+// Trying to get a teeny bit of type-safety where-ever possible, especially for such a critical set of hooks
+func (table *HookTable) Sshook(name string, data string) string {
+	ssHooks, ok := table.Sshooks[name]
+	if ok {
+		for _, hook := range ssHooks {
+			data = hook(data)
+		}
+	}
+	return data
 }
 
 //var vhookErrorable = map[string]func(...interface{}) (interface{}, RouteError){}
@@ -117,12 +211,6 @@ var messageHooks = map[string][]func(Message, PageInt, ...interface{}) interface
 	"topic_reply_row_assign": nil,
 }
 
-// Hooks which take in and spit out a string. This is usually used for parser components
-var Sshooks = map[string][]func(string) string{
-	"preparse_preassign": nil,
-	"parse_assign":       nil,
-}
-
 // The hooks which run before the template is rendered for a route
 var PreRenderHooks = map[string][]func(http.ResponseWriter, *http.Request, *User, interface{}) bool{
 	"pre_render": nil,
@@ -317,87 +405,76 @@ func (plugins PluginList) Load() error {
 // ? - Is this racey?
 // TODO: Generate the cases in this switch
 func (plugin *Plugin) AddHook(name string, handler interface{}) {
+	hookTableUpdateMutex.Lock()
+	defer hookTableUpdateMutex.Unlock()
+
 	switch h := handler.(type) {
 	case func(interface{}) interface{}:
-		if len(Hooks[name]) == 0 {
-			var hookSlice []func(interface{}) interface{}
-			hookSlice = append(hookSlice, h)
-			Hooks[name] = hookSlice
-		} else {
-			Hooks[name] = append(Hooks[name], h)
+		if len(hookTable.Hooks[name]) == 0 {
+			hookTable.Hooks[name] = []func(interface{}) interface{}{}
 		}
-		plugin.Hooks[name] = len(Hooks[name]) - 1
+		hookTable.Hooks[name] = append(hookTable.Hooks[name], h)
+		plugin.Hooks[name] = len(hookTable.Hooks[name]) - 1
 	case func(string) string:
-		if len(Sshooks[name]) == 0 {
-			var hookSlice []func(string) string
-			hookSlice = append(hookSlice, h)
-			Sshooks[name] = hookSlice
-		} else {
-			Sshooks[name] = append(Sshooks[name], h)
+		if len(hookTable.Sshooks[name]) == 0 {
+			hookTable.Sshooks[name] = []func(string) string{}
 		}
-		plugin.Hooks[name] = len(Sshooks[name]) - 1
+		hookTable.Sshooks[name] = append(hookTable.Sshooks[name], h)
+		plugin.Hooks[name] = len(hookTable.Sshooks[name]) - 1
 	case func(http.ResponseWriter, *http.Request, *User, interface{}) bool:
 		if len(PreRenderHooks[name]) == 0 {
-			var hookSlice []func(http.ResponseWriter, *http.Request, *User, interface{}) bool
-			hookSlice = append(hookSlice, h)
-			PreRenderHooks[name] = hookSlice
-		} else {
-			PreRenderHooks[name] = append(PreRenderHooks[name], h)
+			PreRenderHooks[name] = []func(http.ResponseWriter, *http.Request, *User, interface{}) bool{}
 		}
+		PreRenderHooks[name] = append(PreRenderHooks[name], h)
 		plugin.Hooks[name] = len(PreRenderHooks[name]) - 1
 	case func() error: // ! We might want a more generic name, as we might use this signature for things other than tasks hooks
 		if len(taskHooks[name]) == 0 {
-			var hookSlice []func() error
-			hookSlice = append(hookSlice, h)
-			taskHooks[name] = hookSlice
-		} else {
-			taskHooks[name] = append(taskHooks[name], h)
+			taskHooks[name] = []func() error{}
 		}
+		taskHooks[name] = append(taskHooks[name], h)
 		plugin.Hooks[name] = len(taskHooks[name]) - 1
 	case func(...interface{}) interface{}:
-		Vhooks[name] = h
+		hookTable.Vhooks[name] = h
 		plugin.Hooks[name] = 0
 	case func(...interface{}) (bool, RouteError):
-		VhookSkippable[name] = h
+		hookTable.VhookSkippable_[name] = h
 		plugin.Hooks[name] = 0
 	default:
 		panic("I don't recognise this kind of handler!") // Should this be an error for the plugin instead of a panic()?
 	}
+	// TODO: Do this once during plugin activation / deactivation rather than doing it for each hook
+	unsafeRebuildHookTable()
 }
 
 // ? - Is this racey?
 // TODO: Generate the cases in this switch
 func (plugin *Plugin) RemoveHook(name string, handler interface{}) {
+	hookTableUpdateMutex.Lock()
+	defer hookTableUpdateMutex.Unlock()
+
+	key, ok := plugin.Hooks[name]
+	if !ok {
+		panic("handler not registered as hook")
+	}
+
 	switch handler.(type) {
 	case func(interface{}) interface{}:
-		key, ok := plugin.Hooks[name]
-		if !ok {
-			panic("handler not registered as hook")
-		}
-		hook := Hooks[name]
+		hook := hookTable.Hooks[name]
 		if len(hook) == 1 {
 			hook = []func(interface{}) interface{}{}
 		} else {
 			hook = append(hook[:key], hook[key+1:]...)
 		}
-		Hooks[name] = hook
+		hookTable.Hooks[name] = hook
 	case func(string) string:
-		key, ok := plugin.Hooks[name]
-		if !ok {
-			panic("handler not registered as hook")
-		}
-		hook := Sshooks[name]
+		hook := hookTable.Sshooks[name]
 		if len(hook) == 1 {
 			hook = []func(string) string{}
 		} else {
 			hook = append(hook[:key], hook[key+1:]...)
 		}
-		Sshooks[name] = hook
+		hookTable.Sshooks[name] = hook
 	case func(http.ResponseWriter, *http.Request, *User, interface{}) bool:
-		key, ok := plugin.Hooks[name]
-		if !ok {
-			panic("handler not registered as hook")
-		}
 		hook := PreRenderHooks[name]
 		if len(hook) == 1 {
 			hook = []func(http.ResponseWriter, *http.Request, *User, interface{}) bool{}
@@ -406,10 +483,6 @@ func (plugin *Plugin) RemoveHook(name string, handler interface{}) {
 		}
 		PreRenderHooks[name] = hook
 	case func() error:
-		key, ok := plugin.Hooks[name]
-		if !ok {
-			panic("handler not registered as hook")
-		}
 		hook := taskHooks[name]
 		if len(hook) == 1 {
 			hook = []func() error{}
@@ -418,13 +491,15 @@ func (plugin *Plugin) RemoveHook(name string, handler interface{}) {
 		}
 		taskHooks[name] = hook
 	case func(...interface{}) interface{}:
-		delete(Vhooks, name)
+		delete(hookTable.Vhooks, name)
 	case func(...interface{}) (bool, RouteError):
-		delete(VhookSkippable, name)
+		delete(hookTable.VhookSkippable_, name)
 	default:
 		panic("I don't recognise this kind of handler!") // Should this be an error for the plugin instead of a panic()?
 	}
 	delete(plugin.Hooks, name)
+	// TODO: Do this once during plugin activation / deactivation rather than doing it for each hook
+	unsafeRebuildHookTable()
 }
 
 // TODO: Add a HasHook method to complete the AddHook, RemoveHook, etc. set?
@@ -450,55 +525,6 @@ func InitPlugins() {
 }
 
 // ? - Are the following functions racey?
-func RunHook(name string, data interface{}) interface{} {
-	hooks, ok := Hooks[name]
-	if ok {
-		for _, hook := range hooks {
-			data = hook(data)
-		}
-	}
-	return data
-}
-
-func RunHookNoreturn(name string, data interface{}) {
-	hooks, ok := Hooks[name]
-	if !ok {
-		return
-	}
-	for _, hook := range hooks {
-		_ = hook(data)
-	}
-}
-
-// TODO: Use RunHook semantics to allow multiple lined up plugins / modules their turn?
-func RunVhook(name string, data ...interface{}) interface{} {
-	hook := Vhooks[name]
-	if hook != nil {
-		return hook(data...)
-	}
-	return nil
-}
-
-func RunVhookSkippable(name string, data ...interface{}) (bool, RouteError) {
-	return VhookSkippable[name](data...)
-}
-
-func RunVhookNoreturn(name string, data ...interface{}) {
-	hook := Vhooks[name]
-	if hook != nil {
-		_ = hook(data...)
-	}
-}
-
-// TODO: Find a better way of doing this
-func RunVhookNeedHook(name string, data ...interface{}) (ret interface{}, hasHook bool) {
-	hook := Vhooks[name]
-	if hook != nil {
-		return hook(data...), true
-	}
-	return nil, false
-}
-
 func RunTaskHook(name string) error {
 	for _, hook := range taskHooks[name] {
 		err := hook()
@@ -509,17 +535,6 @@ func RunTaskHook(name string) error {
 	return nil
 }
 
-// Trying to get a teeny bit of type-safety where-ever possible, especially for such a critical set of hooks
-func RunSshook(name string, data string) string {
-	ssHooks, ok := Sshooks[name]
-	if ok {
-		for _, hook := range ssHooks {
-			data = hook(data)
-		}
-	}
-	return data
-}
-
 func RunPreRenderHook(name string, w http.ResponseWriter, r *http.Request, user *User, data interface{}) (halt bool) {
 	// This hook runs on ALL PreRender hooks
 	preRenderHooks, ok := PreRenderHooks["pre_render"]

common/group_store.go

@@ -229,7 +229,7 @@ func (mgs *MemoryGroupStore) Create(name string, tag string, isAdmin bool, isMod
 	var blankIntList []int
 	var pluginPerms = make(map[string]bool)
 	var pluginPermsBytes = []byte("{}")
-	RunVhook("create_group_preappend", &pluginPerms, &pluginPermsBytes)
+	GetHookTable().Vhook("create_group_preappend", &pluginPerms, &pluginPermsBytes)
 
 	// Generate the forum permissions based on the presets...
 	fdata, err := Forums.GetAll()

common/pages.go

@@ -23,6 +23,7 @@ type Header struct {
 	Theme       *Theme
 	//TemplateName string // TODO: Use this to move template calls to the router rather than duplicating them over and over and over?
 	CurrentUser User // TODO: Deprecate CurrentUser on the page structs and use a pointer here
+	Hooks       *HookTable
 	Zone        string
 	Path        string
 	MetaDesc    string
@@ -50,6 +51,7 @@ func (header *Header) AddNotice(name string) {
 type HeaderLite struct {
 	Site     *site
 	Settings SettingMap
+	Hooks    *HookTable
 	ExtData  ExtData
 }
 

common/parser.go

@@ -203,7 +203,7 @@ func PreparseMessage(msg string) string {
 	msg = strings.Replace(msg, " ", "", -1)
 	msg = strings.Replace(msg, "\r", "", -1) // Windows artifact
 	//msg = strings.Replace(msg, "\n\n\n\n", "\n\n\n", -1)
-	msg = RunSshook("preparse_preassign", msg)
+	msg = GetHookTable().Sshook("preparse_preassign", msg)
 	// There are a few useful cases for having spaces, but I'd like to stop the WYSIWYG from inserting random lines here and there
 	msg = SanitiseBody(msg)
 
@@ -624,7 +624,7 @@ func ParseMessage(msg string, sectionID int, sectionType string /*, user User*/)
 	}
 
 	msg = strings.Replace(msg, "\n", "<br>", -1)
-	msg = RunSshook("parse_assign", msg)
+	msg = GetHookTable().Sshook("parse_assign", msg)
 	return msg
 }
 

common/routes_common.go

@@ -20,28 +20,29 @@ var ForumUserCheck func(w http.ResponseWriter, r *http.Request, user *User, fid
 var SimpleUserCheck func(w http.ResponseWriter, r *http.Request, user *User) (headerLite *HeaderLite, err RouteError) = simpleUserCheck
 var UserCheck func(w http.ResponseWriter, r *http.Request, user *User) (header *Header, err RouteError) = userCheck
 
-func simpleForumUserCheck(w http.ResponseWriter, r *http.Request, user *User, fid int) (headerLite *HeaderLite, rerr RouteError) {
+func simpleForumUserCheck(w http.ResponseWriter, r *http.Request, user *User, fid int) (header *HeaderLite, rerr RouteError) {
+	header, rerr = SimpleUserCheck(w, r, user)
+	if rerr != nil {
+		return header, rerr
+	}
 	if !Forums.Exists(fid) {
 		return nil, PreError("The target forum doesn't exist.", w, r)
 	}
 
 	// Is there a better way of doing the skip AND the success flag on this hook like multiple returns?
-	if VhookSkippable["simple_forum_check_pre_perms"] != nil {
-		var skip bool
-		skip, rerr = RunVhookSkippable("simple_forum_check_pre_perms", w, r, user, &fid, &headerLite)
-		if skip || rerr != nil {
-			return headerLite, rerr
-		}
+	skip, rerr := header.Hooks.VhookSkippable("simple_forum_check_pre_perms", w, r, user, &fid, &header)
+	if skip || rerr != nil {
+		return header, rerr
 	}
 
 	fperms, err := FPStore.Get(fid, user.Group)
 	if err == ErrNoRows {
 		fperms = BlankForumPerms()
 	} else if err != nil {
-		return headerLite, InternalError(err, w, r)
+		return header, InternalError(err, w, r)
 	}
 	cascadeForumPerms(fperms, user)
-	return headerLite, nil
+	return header, nil
 }
 
 func forumUserCheck(w http.ResponseWriter, r *http.Request, user *User, fid int) (header *Header, rerr RouteError) {
@@ -53,12 +54,9 @@ func forumUserCheck(w http.ResponseWriter, r *http.Request, user *User, fid int)
 		return header, NotFound(w, r, header)
 	}
 
-	if VhookSkippable["forum_check_pre_perms"] != nil {
-		var skip bool
-		skip, rerr = RunVhookSkippable("forum_check_pre_perms", w, r, user, &fid, &header)
-		if skip || rerr != nil {
-			return header, rerr
-		}
+	skip, rerr := header.Hooks.VhookSkippable("forum_check_pre_perms", w, r, user, &fid, &header)
+	if skip || rerr != nil {
+		return header, rerr
 	}
 
 	fperms, err := FPStore.Get(fid, user.Group)
@@ -99,7 +97,6 @@ func cascadeForumPerms(fperms *ForumPerms, user *User) {
 // TODO: Do a panel specific theme?
 func panelUserCheck(w http.ResponseWriter, r *http.Request, user *User) (header *Header, stats PanelStats, rerr RouteError) {
 	var theme = &Theme{Name: ""}
-
 	cookie, err := r.Cookie("current_theme")
 	if err == nil {
 		inTheme, ok := Themes[html.EscapeString(cookie.Value)]
@@ -117,6 +114,7 @@ func panelUserCheck(w http.ResponseWriter, r *http.Request, user *User) (header
 		Themes:      Themes,
 		Theme:       theme,
 		CurrentUser: *user,
+		Hooks:       GetHookTable(),
 		Zone:        "panel",
 		Writer:      w,
 	}
@@ -155,7 +153,7 @@ func panelUserCheck(w http.ResponseWriter, r *http.Request, user *User) (header
 }
 
 func simplePanelUserCheck(w http.ResponseWriter, r *http.Request, user *User) (headerLite *HeaderLite, rerr RouteError) {
-	return simpleUserCheck(w, r, user)
+	return SimpleUserCheck(w, r, user)
 }
 
 // SimpleUserCheck is back from the grave, yay :D
@@ -163,6 +161,7 @@ func simpleUserCheck(w http.ResponseWriter, r *http.Request, user *User) (header
 	return &HeaderLite{
 		Site:     Site,
 		Settings: SettingBox.Load().(SettingMap),
+		Hooks:    GetHookTable(),
 	}, nil
 }
 
@@ -187,6 +186,7 @@ func userCheck(w http.ResponseWriter, r *http.Request, user *User) (header *Head
 		Themes:      Themes,
 		Theme:       theme,
 		CurrentUser: *user, // ! Some things rely on this being a pointer downstream from this function
+		Hooks:       GetHookTable(),
 		Zone:        "frontend",
 		Writer:      w,
 	}

common/topic_list.go

@@ -258,7 +258,7 @@ func (tList *DefaultTopicList) getList(page int, orderby string, argList []inter
 		topicItem.RelativeLastReplyAt = RelativeTime(topicItem.LastReplyAt)
 
 		// TODO: Rename this Vhook to better reflect moving the topic list from /routes/ to /common/
-		RunVhook("topics_topic_row_assign", &topicItem, &forum)
+		GetHookTable().Vhook("topics_topic_row_assign", &topicItem, &forum)
 		topicList = append(topicList, &topicItem)
 		reqUserList[topicItem.CreatedBy] = true
 		reqUserList[topicItem.LastReplyBy] = true

langs/english.json

@@ -121,6 +121,7 @@
 		"page":"Page",
 		"topics":"All Topics",
 		"forums":"Forum List",
+		"create_topic":"Create Topic",
 		"login":"Login",
 		"login_mfa_verify":"2FA Verify",
 		"register":"Registration",

misc_test.go

@@ -967,14 +967,23 @@ func TestPluginManager(t *testing.T) {
 	expectNilErr(t, plugin.SetActive(false))
 
 	// Hook tests
-	expect(t, common.RunSshook("haha", "ho") == "ho", "Sshook shouldn't have anything bound to it yet")
-	var handle = func(in string) (out string) {
+	expect(t, common.GetHookTable().Sshook("haha", "ho") == "ho", "Sshook shouldn't have anything bound to it yet")
+	handle := func(in string) (out string) {
 		return in + "hi"
 	}
 	plugin.AddHook("haha", handle)
-	expect(t, common.RunSshook("haha", "ho") == "hohi", "Sshook didn't give hohi")
+	expect(t, common.GetHookTable().Sshook("haha", "ho") == "hohi", "Sshook didn't give hohi")
 	plugin.RemoveHook("haha", handle)
-	expect(t, common.RunSshook("haha", "ho") == "ho", "Sshook shouldn't have anything bound to it anymore")
+	expect(t, common.GetHookTable().Sshook("haha", "ho") == "ho", "Sshook shouldn't have anything bound to it anymore")
+
+	expect(t, common.GetHookTable().Hook("haha", "ho") == "ho", "Hook shouldn't have anything bound to it yet")
+	handle2 := func(inI interface{}) (out interface{}) {
+		return inI.(string) + "hi"
+	}
+	plugin.AddHook("hehe", handle2)
+	expect(t, common.GetHookTable().Hook("hehe", "ho").(string) == "hohi", "Hook didn't give hohi")
+	plugin.RemoveHook("hehe", handle2)
+	expect(t, common.GetHookTable().Hook("hehe", "ho").(string) == "ho", "Hook shouldn't have anything bound to it anymore")
 
 	// TODO: Add tests for more hook types
 }

plugin_test.go

@@ -5,6 +5,8 @@ import "testing"
 
 // go test -v
 
+// TODO: Write a test for Hello World?
+
 type MEPair struct {
 	Msg     string
 	Expects string

routes/forum.go

@@ -81,7 +81,7 @@ func ViewForum(w http.ResponseWriter, r *http.Request, user common.User, sfid st
 		topicItem.Link = common.BuildTopicURL(common.NameToSlug(topicItem.Title), topicItem.ID)
 		topicItem.RelativeLastReplyAt = common.RelativeTime(topicItem.LastReplyAt)
 
-		common.RunVhookNoreturn("forum_trow_assign", &topicItem, &forum)
+		header.Hooks.VhookNoRet("forum_trow_assign", &topicItem, &forum)
 		topicList = append(topicList, &topicItem)
 		reqUserList[topicItem.CreatedBy] = true
 		reqUserList[topicItem.LastReplyBy] = true

routes/forum_list.go

@@ -47,7 +47,7 @@ func ForumList(w http.ResponseWriter, r *http.Request, user common.User) common.
 			} else {
 				forum.LastTopicTime = ""
 			}
-			common.RunHook("forums_frow_assign", &forum)
+			header.Hooks.Hook("forums_frow_assign", &forum)
 			forumList = append(forumList, forum)
 		}
 	}

routes/reports.go

@@ -10,7 +10,7 @@ import (
 )
 
 func ReportSubmit(w http.ResponseWriter, r *http.Request, user common.User, sitemID string) common.RouteError {
-	_, ferr := common.SimpleUserCheck(w, r, &user)
+	headerLite, ferr := common.SimpleUserCheck(w, r, &user)
 	if ferr != nil {
 		return ferr
 	}
@@ -67,7 +67,7 @@ func ReportSubmit(w http.ResponseWriter, r *http.Request, user common.User, site
 		title = "Topic: " + topic.Title
 		content = topic.Content + "\n\nOriginal Post: #tid-" + strconv.Itoa(itemID)
 	} else {
-		_, hasHook := common.RunVhookNeedHook("report_preassign", &itemID, &itemType)
+		_, hasHook := headerLite.Hooks.VhookNeedHook("report_preassign", &itemID, &itemType)
 		if hasHook {
 			return nil
 		}

routes/topic.go

@@ -186,7 +186,7 @@ func ViewTopic(w http.ResponseWriter, r *http.Request, user common.User, urlBit
 				likedQueryList = append(likedQueryList, replyItem.ID)
 			}
 
-			common.RunVhookNoreturn("topic_reply_row_assign", &tpage, &replyItem)
+			header.Hooks.VhookNoRet("topic_reply_row_assign", &tpage, &replyItem)
 			// TODO: Use a pointer instead to make it easier to abstract this loop? What impact would this have on escape analysis?
 			tpage.ItemList = append(tpage.ItemList, replyItem)
 		}
@@ -235,6 +235,7 @@ func ViewTopic(w http.ResponseWriter, r *http.Request, user common.User, urlBit
 // TODO: Add a permission to stop certain users from using custom avatars
 // ? - Log username changes and put restrictions on this?
 // TODO: Test this
+// TODO: Revamp this route
 func CreateTopic(w http.ResponseWriter, r *http.Request, user common.User, sfid string) common.RouteError {
 	var fid int
 	var err error
@@ -256,13 +257,13 @@ func CreateTopic(w http.ResponseWriter, r *http.Request, user common.User, sfid
 		return common.NoPermissions(w, r, user)
 	}
 	// TODO: Add a phrase for this
-	header.Title = "Create Topic"
+	header.Title = common.GetTitlePhrase("create_topic")
 	header.Zone = "create_topic"
 
 	// Lock this to the forum being linked?
 	// Should we always put it in strictmode when it's linked from another forum? Well, the user might end up changing their mind on what forum they want to post in and it would be a hassle, if they had to switch pages, even if it is a single click for many (exc. mobile)
 	var strictmode bool
-	common.RunVhookNoreturn("topic_create_pre_loop", w, r, fid, &header, &user, &strictmode)
+	header.Hooks.VhookNoRet("topic_create_pre_loop", w, r, fid, &header, &user, &strictmode)
 
 	// TODO: Re-add support for plugin_guilds
 	var forumList []common.Forum
@@ -295,11 +296,8 @@ func CreateTopic(w http.ResponseWriter, r *http.Request, user common.User, sfid
 		if forum.Name != "" && forum.Active {
 			fcopy := forum.Copy()
 			// TODO: Abstract this
-			if common.Hooks["topic_create_frow_assign"] != nil {
-				// TODO: Add the skip feature to all the other row based hooks?
-				if common.RunHook("topic_create_frow_assign", &fcopy).(bool) {
-					continue
-				}
+			if header.Hooks.HookSkippable("topic_create_frow_assign", &fcopy) {
+				continue
 			}
 			forumList = append(forumList, fcopy)
 		}

templates/create_topic.html

@@ -3,7 +3,7 @@
 	<div class="rowblock rowhead">
 		<div class="rowitem"><h1>{{lang "create_topic_head"}}</h1></div>
 	</div>
-	<div class="rowblock">
+	<div class="rowblock the_form">
 		<form id="quick_post_form" enctype="multipart/form-data" action="/topic/create/submit/?session={{.CurrentUser.Session}}" method="post"></form>
 		<div class="formrow real_first_child">
 			<div class="formitem formlabel"><a>{{lang "create_topic_board"}}</a></div>