hkzorman / advanced_npc

Advanced NPC for Minetest, using mobs_redo API
Other
18 stars 5 forks source link

Unify actions/tasks API #36

Closed hkzorman closed 6 years ago

hkzorman commented 6 years ago

Unified Commands API

Description:

The Commands API is a Bash Script-like execution environment for Minetest entities. There are the following fundamental constructs:

New commands

The following commands will be added to the default set:

If/else:

This command allows the conditional execution of two array of commands depending on the evaluation of a certain condition. This is the typical if-else statement of a programming language. If-else can be nested. Arguments:

Loops:

This command works as the types of loops, depending on the arguments given. It can work as a while, a for and a for-each loop. Loops can be nested. While-loop arguments:

Extensibility

Once the above commands has been added, it is possible to safely build scripts which don't touch directly many of the internal NPC mechanisms. An API will be provided for external mods to register scripts that let NPCs perform actions related to those mods, e.g. operating a node provided by the mod. The API for this will be:

npc.commands.register_script(name, script)

All registered scripts have the following properties:

The script parameter is a Lua array of commands that will be executed when the script is executed.

BrunoMine commented 6 years ago

This is a big change, is it already implemented?

hkzorman commented 6 years ago

This is a proposal for unifying the actions API, what do you think about it?

BrunoMine commented 6 years ago

look great. I recommend that you keep a well-organized and documented code so that I can contribute as well.

hkzorman commented 6 years ago

Will do!

hkzorman commented 6 years ago

As this is a big piece of work, I have created a separate branch (https://github.com/hkzorman/advanced_npc/tree/commands_api) to implement this. I have started pushing initial code. It is still WIP and not usable. Will soon open a PR.

hkzorman commented 6 years ago

Updated the first post, more in accordance to the in-progress implementation.

hkzorman commented 6 years ago

I'm seriously considering the need to implement an if/else and for loop commands. The reason I started with this was because I wanted to allow other mods to register scripts (lists of commands) that the NPC would follow, and didn't wanted direct access from these mods to the self.* in an attempt to consider some sort of security. However, considering this well, I don't believe that encapsulating that functionality in commands will actually help so much with security as to out-weight the benefits of allowing to register pure Lua functions.

What are your opinions? As an example, the original idea (with commands) attempted something like this (example below is looking for players and changing the yaw to look at it):

npc.commands.register_script("look_for_player", {
  [1] = npc.commands.loop({condition = "true", commands = {
          [1] = npc.commands.query({type="entities", radius="2", name="players"})
          [2] = npc.commands.if_else({condition="players > 0", commands = {
                  [1] = npc.commands.rotate({dir=minetest.dir_to_yaw(players[0])})
              })
      })
}

Already the difficulties can be seen. While with a pure Lua function it can be done like this:

npc.commands.register_script("look_for_player", function(self) {
  while true do
     local players = minetest.get_objects_in_radius(self.object:getpos(), 2)
     if #players > 0 then
       ...
     end
  end
})

Looks to me like much more practical and little security risk. If anyone can think a negative thing about adding this npc.commands.register_script() with pure Lua functions and full access to the self variable, please comment here.

BrunoMine commented 6 years ago

Commands may seem too complex for some situations, but I believe they will be needed for more complex activities. For example, walking to a place or cooking an item, as these tasks require several steps, this will become clearer and simpler if they are encapsulated. Do you agree?

hkzorman commented 6 years ago

I agree, things like walking to a position or using furnace will remain builtin. These will be called now scripts instead of tasks. Will come up with a new doc soon.

BrunoMine commented 6 years ago

The commands will also make the development of NPCs more efficient over time, for example, suppose that the minetest engine creates a new more agile method for locating nearby players (a very hypothetical example), the mods that use the command "look_for_player" will not need to be updated, only the API will take care of making everything more efficient. What I mean, commands will also allow the API to become increasingly specialized in making NPCs smart, without mod's developers having to worry about it.

hkzorman commented 6 years ago

Ok I understand your vision of this. In general, you would like the NPC API to provide default commands/scripts, whatever they are called, that will perform general enough actions. For example:

I agree with you, and I will implement these extra functions. I also was thinking of providing other mods the ability to define a command/script in case their functionality was too specialized. For example, think a music player. A player has to right click to place a disc and punch to play it. If this mod wanted a NPC to operate this player, it wouldn't be possible with default API. However, if we allow mods to register scripts to suite their own needs, then NPCs will be able to do almost everything as long as external mods support this mod.

BrunoMine commented 6 years ago

Certainly, it is necessary to allow the creation of custom commands for very specific situations. This was one of my biggest issues when implementing advanced_npc in my projects.

hkzorman commented 6 years ago

Excellent, then I will add these functionality. I guess the direction that the improvement of the commands API has taken now is different so I will start again.

BrunoMine commented 6 years ago

Yes, it will change a bit, to make it more semantic to the external developer.

hkzorman commented 6 years ago

During this time I have been figuring out the direction this should take. These are the things I have come up with:

I think this is enough, it allows lot of flexibility and lots of functionality to be built, but I'm a little bit concerned about the ease of writing scripts, specially scripts that need to execute multiple functions over time (such as follow). Such scripts need to be recursive, and implementing them might be a little bit hard.

Here is the follow script I have implemented for advanced_npc. It works, completely, at least for following players. It works better than the mobs_redo follow, and I'm sure I can make it smarter than it is right now. However, please comment what you think about it.

-- Follow state script. This is a looping script that will try to follow an
-- entity or player until either of the following conditions are met:
--   - A certain flag is set to false
--   - The object is reached and a callback executed.
-- This script is not interruptable by the scheduler function.
-- Arguments:
--   - `radius`: integer, initial search radius. Default is 3
--   - `max_radius`: integer, maximum search radius. If target isn't found within initial radius,
--      radius will increase up to this value. Default is 20
--   - `speed`: number, walking speed for the NPC while following. Default is 3
--   - `target`: string, can be "player" or "entity".
--   - `player_name`: string, name of player to follow
--   - `entity_type`: string, type of entity to follow. NOT IMPLEMENTED.
--   - `on_reach`: function, if given, on reaching the target, this function will
--      be called and executed. On execution, the script will finish. DO NOT use with
--      `follow_flag`.
--   - `follow_flag`: string, flag name. If given, the script will keep running until the
--      value of this flag is false. DO NOT use with `on_reach`.
npc.commands.register_script("advanced_npc:follow", false, function(self, args)
    local radius = args.radius or 3
    local max_radius = args.max_radius or 20
    local speed = args.follow_speed or 3
    local target = args.target
    local player_name = args.player_name
    local on_reach = args.on_reach
    local follow_flag = args.follow_flag
    local results_key = "player_follow"

    -- Run this 1/speed times in a second
    npc.enqueue_command(self, npc.commands.cmd.SET_INTERVAL, {interval=1/speed, freeze=true})
    -- Make NPC climb one-block heights. Makes following easier
    self.stepheight = 1.1
    self.object:set_properties(self)

    -- Function used to reset NPC values once following is complete
    local reset = function(self)
        self.stepheight = 0.6
        self.object:set_properties(self)
        npc.enqueue_command(self, npc.commands.cmd.SET_INTERVAL, {interval=1, freeze=false})
    end

    -- Script functions
    local follow_player = function(self, args)
        if args.target == "player" then
            local player_name = args.player_name
            local objs = minetest.get_objects_inside_radius(self.object:getpos(), args.radius)
            -- Check if objects were found
            minetest.log("Objects found: "..dump(objs))
            if #objs > 0 then
                for _,obj in pairs(objs) do
                    if obj then
                        -- Check if this is the player we are looking for
                        if obj:is_player() and obj:get_player_name() == player_name then
                            local target_pos = vector.round(obj:getpos())
                            -- Calculate distance - if less than 3, avoid walking any further
                            if vector.distance(self.object:getpos(), target_pos) < 3 then
                                npc.log("SCRIPT", "[follow] Destination reached")
                                -- Destination reached
                                -- Add standing action if NPC is still moving
                                if math.abs(vector.length(self.object:getvelocity())) > 0 then
                                    npc.enqueue_command(self, npc.commands.cmd.STAND,
                                        {dir = npc.commands.get_direction(self.object:getpos(), target_pos)}
                                    )
                                end

                                -- Execute `on_reach` function if present
                                if on_reach then
                                    npc.log("SCRIPT", "[follow] Executing on_reach callback...")
                                    on_reach(self, obj)
                                    return {reached_target = true, target_pos = target_pos, end_execution = true}
                                end

                                return {reached_target = true, target_pos = target_pos}
                            else
                                npc.log("SCRIPT", "[follow] Walking towards player...")
                                local walk_args = {
                                    dir = npc.commands.get_direction(self.object:getpos(), target_pos),
                                    speed = speed
                                }
                                -- Enqueue walk step
                                npc.enqueue_command(self, npc.commands.cmd.WALK_STEP, walk_args)
                                return {reached_target = false, target_pos = target_pos}
                            end
                        end
                    end
                end
                -- Player not found, stop
                npc.enqueue_command(self, npc.commands.cmd.STAND, {})
                return {reached_target = false, target_pos = nil}
            end
        end
        return {reached_target = false, target_pos = nil}
    end

    local check_if_complete

    check_if_complete = function(self, args)
        -- Check if follow is still needed
        if npc.get_flag(self, follow_flag) == false then
            -- Stop, follow no more
            npc.enqueue_command(self, npc.commands.cmd.STAND, {})
            -- Clear flag
            npc.update_flag(self, follow_flag, nil)
            -- Reset actions interval and NPC stepheight
            reset(self)
            return
        end

        -- Get results from following
        local follow_result = npc.execution_context.get(self, args.results_key)
        -- Check results
        if follow_result == nil then
            npc.log("WARNING", "Unable to find result in execution context for 'follow_player' function using key: "..
            dump(results_key))
            return
        end
        -- Clean execution context
        npc.execution_context.remove(self, args.results_key)

        -- Check if target reached and on_reach function executed
        if follow_result.reached_target == true and follow_result.end_execution == true then
            return
        end
        -- on_reach is not set, keep executing until follow flag is off.
        if follow_result.target_pos ~= nil then
            -- Keep walking or waiting for player to keep moving
            npc.enqueue_function(self, follow_player, {
                target=args.target,
                radius=args.radius,
                player_name=args.player_name
            }, results_key)
            -- Check if follow is complete
            npc.enqueue_function(self, check_if_complete, {
                results_key = results_key,
                target = args.target,
                radius = args.radius,
                player_name = args.player_name})
            --npc.enqueue_function(self, detect_more_movement, {player_pos = follow_result.target_pos})
        else
            -- Cannot find
            npc.log("SCRIPT", "[follow] Walking towards player")
            npc.enqueue_function(self, follow_player, {
                target=args.target,
                radius=args.radius + 1,
                player_name=args.player_name}, results_key)
            -- Check if follow is complete
            npc.enqueue_function(self, check_if_complete, {
                results_key = results_key,
                target = args.target,
                radius = args.radius + 1,
                player_name = args.player_name})
        end
    end

    -- Execution
    -- Follow, results to be stored on execution context with key "results_key"
    npc.enqueue_function(self, follow_player, {
        target = player_name,
        radius = radius,
        player_name = player_name
    }, results_key)
    -- Check if follow is complete
    npc.enqueue_function(self, check_if_complete, {
        results_key = results_key,
        target = target,
        radius = radius,
        player_name = player_name})
end)

The basic structure is, a function that follows the player and a function that checks what to do next, and then enqueue again the follow_player function and the check function. It is repeating itself by enqueuing itself again. Is this a hard concept to understand? Could there be a better way to do this, rather than enqueuing function many times?

BrunoMine commented 6 years ago

I've been away for a while, I'm waiting for the new version of minetest to translate online mods, but I'll continue the implementation of advanced_npc in my mod. Is it possible to use it the way it is currently? It's safe?

hkzorman commented 6 years ago

Hey @BrunoMine , good to see you around again. I've done lots and lots of (yet uncomitted) changes in the background, which I will be opening a new branch again once I fix the mess in my local repo.

I would recommend to stay away from head and only support https://github.com/hkzorman/advanced_npc/releases/tag/1.0.0-alpha-1 , as head seems to be unstable and will change anyways soon.

Now that you are around again I will be more motivated to actually push my code to a new branch... I can tell you that I have changed the actions/task/scripts model completely and implemented a real OS-like design. We have now programs, processes, a process queue, a process scheduler and the previous job-scheduling system (no multi-programming though). I'm done with implementation and currently fixing bugs.

Hopefully the changes in your mod shouldn't be that big. I can help you once you start working on that.

hkzorman commented 6 years ago

Actually, I've just committed my changes. It might not be 100% but you can have a look at the API, for that I expect no changes. https://github.com/hkzorman/advanced_npc/tree/commands_api?files=1

BrunoMine commented 6 years ago

Is the documentation completely up to date with these recent improvements?

hkzorman commented 6 years ago

@BrunoMine unfortunately, no. I will try to take some time in the next days to document most of it.

BrunoMine commented 6 years ago

Then, can I already work with commands_api?

hkzorman commented 6 years ago

Feel free to give it a try! It is mostly stable on my local and I've ported all old functionality to the new API.

You can report any bugs and I will fix them as soon as I can!

As I said before, the execution API (npc.exec) is stable and shouldn't change.

hkzorman commented 6 years ago

Just don't expect it to be as the original post here, this needs to change. The API follows an OS like model where "programs" (equivalent to tasks in the past) are registered, programs execute "instructions" (equivalent to actions in the past) and when a program is run it is put into a process queue (which is handled by a process scheduler that runs every 1 second). There is also the ability to interrupt processes in which the process state is stored. Also, processes have a variable storage if they need them.

BrunoMine commented 6 years ago

You really did a great work here. I'm going to invest some time appreciating this in my mods.

hkzorman commented 6 years ago

Implemented by #44