hkzorman / advanced_npc

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

Make advanced_npc a pure API mod #53

Closed hkzorman closed 6 years ago

hkzorman commented 6 years ago

Fixes #53

This pull request will remove any data definition or NPC-specific code implementation and finally generalize advanced_npc to make it into a true API.

Task list:

More will be added as I understand what needs to be done in order to complete on this.

hkzorman commented 6 years ago

Hey @BrunoMine, could you please test this branch? I'm afraid it might break something in your mod.

hkzorman commented 6 years ago

Actually, I downloaded your mod and tested. Fixed two issues, however, I'm not sure if everything is correct, I strongly recommend you to look at the NPC behavior and make sure they are still working properly (I saw one NPC switching back and forth between the kit culinario and crafting table, while I saw other that was very very still (never walked around). I'm not sure if that is the intended behavior or not, so please check.

And btw, congratulations on your mod so far! We seriously need mods that pay attention to details, and I can see yours is heading in that direction. I really like the houses, their layout, and the landscape around. Hopefully we can have smart NPCs living in those houses!

BrunoMine commented 6 years ago

The NPC behavior must be alternating between objects randomly. (dev brach) I will test this.

BrunoMine commented 6 years ago

The NPCs are using a wrong texture.

BrunoMine commented 6 years ago

I realized that you're trying to deploy a tag system for textures, so I adjusted my textures, but you need to document and fix the problem. To test with my mod use: dev brach Minetest 0.5.0 (dev version) Flat world seed

BrunoMine commented 6 years ago

Another problem detected. Apparently he is not running all the instructions in the queue. See this code:

npc.programs.instr.register("sunos:rotate_to_pos", function(self, args)
    minetest.chat_send_all("sunos:rotate_to_pos")
    npc.programs.instr.execute(self, "advanced_npc:rotate", {
        start_pos = self.object:getpos(), 
        end_pos = args.pos,
    })
end)

npc.programs.instr.register("sunos:set_animation", function(self, args)
    minetest.chat_send_all("sunos:set_animation")
    self.object:set_properties({mesh = args.mesh})
    self.object:set_animation(
        {x = args.start_frame, y = args.end_frame},
            args.frame_speed, 
            0
        )
end)

-- Interagir aleatoriamente com a mobilia da casa
npc.programs.register("sunos:interagir_mobilia", function(self, args)

    -- Verificar distancia de casa
    if verif_dist_pos(self.object:getpos(), self.sunos_fundamento) > 16 then
        return
    end

    local places = npc.locations.get_by_type(self, "mobilia")

    p = places[math.random(1, #places)]

    npc.locations.add_shared(self, "sunos_alvo_mobilia", "sunos_alvo_mobilia", p.pos, p.access_node)

    npc.exec.proc.enqueue(self, "advanced_npc:interrupt", {
        new_program = "advanced_npc:walk_to_pos",
        new_args = {
            end_pos = {
                place_type="sunos_alvo_mobilia",
                use_access_node=true
            },
            walkable = sunos.estruturas.casa.walkable_nodes
        },
        interrupt_options = {}
    })

    -- Vira para "pos"
    npc.exec.proc.enqueue(self, "sunos:rotate_to_pos", {
        pos = p.pos,
    })

    -- Muda animação
    npc.exec.proc.enqueue(self, "sunos:set_animation", {
        mesh = "sunos_movimento_bancada.b3d",
        start_frame = 20,
        end_frame = 60,
        frame_speed = 25,
    })

    -- Fica parado por um tempo
    npc.exec.proc.enqueue(self, "advanced_npc:wait", {
        time = 5,
    })

end)

I only get the sunos:set_animation message and I never get sunos:rotate_to_pos. EDIT: In the master branch it continued working normally.

hkzorman commented 6 years ago

The texture issue is fixed, and opened a PR on your repo https://github.com/BrunoMine/sociedades/pull/8 I will check the queue issue next.

hkzorman commented 6 years ago

As you suggested, one of the instructions in queue was being dequeued before time. This is now fixed in this branch.

Your mod seems to be working good. They are very hard workers!

BrunoMine commented 6 years ago

I'm still just testing this, I need to remember the blender modeler. XD I have oberserved two problems:

hkzorman commented 6 years ago

The advanced_npc:wait issue I have noticed, just committed a fix for it... should work. The other I'm not sure what you mean. In what conditions it doesn't walk? When the distance between NPC and node is less than 2?

BrunoMine commented 6 years ago

Exact

hkzorman commented 6 years ago

This is on purpose, when I optimized the program to avoid extra walks, it will just rotate toward the target node. If you want I can add an option to avoid this and walk anyways... not sure how it will work though.

BrunoMine commented 6 years ago

This optimized program is very nice, is most common cases. But in this specific case I need that the NPC walk for a exact pos. Pls add this opition.

hkzorman commented 6 years ago

Last push in this branch includes optional argument: optimize_one_node_distance which is default to true. Set it to false and let's see what happens... haven't tested.

PR closing was accidental.

BrunoMine commented 6 years ago

Thanks, I will continue my development, if I find any new problem I will inform. I will make progress with the documentation as well.

hkzorman commented 6 years ago

Do not worry about docs... if you are referring to Info API, this one is not stable enough to start working on documentation.

BrunoMine commented 6 years ago

okay. Apparently, the walk_to_posproblem still occurs when the node is diagonal to the NPC (It only rotate the NPC). I need the NPC to stay in the exact position.

BrunoMine commented 6 years ago

Another problem detected. The NPC is not obeying the schedule. To test, I set the time to 0 and checked the NPC, it keeps working.

-- Registra ocupação padrão no NPC caseiro
npc.occupations.register_occupation("sunos_npc_caseiro", {
    dialogues = {},
    textures = {
        {name="sunos_npc_male.png", tags={"male", "adult", "sunos_npc_caseiro"}},
        {name="sunos_npc_female.png", tags={"female", "adult", "sunos_npc_caseiro"}}
    },
    building_types = {},
    surrounding_building_types = {},
    walkable_nodes = sunos.estruturas.casa.walkable_nodes,
    initial_inventory = {},
    schedules_entries = sunos.copy_tb({
        -- Durmir
        [0] = sunos.estruturas.casa.durmir,
        [1] = sunos.estruturas.casa.durmir,
        [2] = sunos.estruturas.casa.durmir,
        [3] = sunos.estruturas.casa.durmir,
        [4] = sunos.estruturas.casa.durmir,
        [5] = sunos.estruturas.casa.durmir,
        -- Mecher em casa
        [6] = interagir_casa,
        [7] = interagir_casa,
        [8] = interagir_casa,
        [9] = interagir_casa,
        [10] = interagir_casa,
        [11] = interagir_casa,
        [12] = interagir_casa,
        [13] = interagir_casa,
        [14] = interagir_casa,
        [15] = interagir_casa,
        [16] = interagir_casa,
        [17] = interagir_casa,
        [18] = interagir_casa,
        [19] = interagir_casa,
        [20] = interagir_casa,
        [21] = interagir_casa,
        -- Durmir
        [22] = sunos.estruturas.casa.durmir,
        [23] = sunos.estruturas.casa.durmir
    })  
})
hkzorman commented 6 years ago

I have been working on the walk_to_pos issue, still not having favorable results.

Regarding the schedules, I have performed a little change that changes the way they work. Before you could do /time 6000 or any value and the schedule would change. I have done a change were the NPC won't process a schedule if its time is less than the previously processed time, e.g. if time is 9000, and you try /time 6000 it won't work, it only goes forward. The only exception here is 23999 -> 0, which is allowed. Try changing time to 22950, let the log print Time 23 and then set time to 23950. I think it should work.

hkzorman commented 6 years ago

Added a small change... please tell me if it improves, I'm currently unable to test properly. Also, remember that in your walk_to_pos have the argument optimize_one_node_distance set to false.

BrunoMine commented 6 years ago

This works at first, but after a while I noticed that the NPC does not wake up. I believe there is a bug, but it can be difficult to find. I took pictures and opened new issues to resolve these bugs

hkzorman commented 6 years ago

Thank you, I will be taking a look at them. I currently have less time to work on the mod but will be addressing them soon.

BrunoMine commented 6 years ago

I'm developing all my mod based on that branch. As soon as I finish the next version (of my mod), I believe it will be stable enough to be released (this mod).

hkzorman commented 6 years ago

I'm also developing my mg_villages_npc mod with this branch, and there are few things that I change as I use it from an outside perspective. Please let me know when you are done with your mod.

BrunoMine commented 6 years ago

I think I'm ready to release the next version of my mod. I will leave some things for the future releases (like dialogues), because I intend to wait until the basic problems and needs are completely solved.

BrunoMine commented 6 years ago

What should be the next step in your opnion? Launch the next version of the two projects or just mine project (in this case I will need to inform a dependency for the specific branch in your repository)? I believe that many other problems may appear after my release.

BrunoMine commented 6 years ago

After we solve all that. I'm going to propose the improvements that I'm already using in my project in a modular way (I'm trying to keep the code working in parallel to send PR in the future).

BrunoMine commented 6 years ago

Can you make this compatible with the previous method?

2018-06-22 20:25:53: ERROR[Main]: ServerError: AsyncErr: ServerThread::run Lua: Runtime error from mod 'sunos' in callback luaentity_Activate(): ...Mods/minetest 0-5-0/bin/../mods/advanced_npc/npc.lua:300: attempt to index local 'npc_info' (a nil value)
2018-06-22 20:25:53: ERROR[Main]: stack traceback:
2018-06-22 20:25:53: ERROR[Main]:   ...Mods/minetest 0-5-0/bin/../mods/advanced_npc/npc.lua:300: in function 'initialize'
2018-06-22 20:25:53: ERROR[Main]:   ...etest 0-5-0/bin/../mods/sociedades/sunos/npc/npc.lua:308: in function 'on_spawn'
2018-06-22 20:25:53: ERROR[Main]:   ...de Mods/minetest 0-5-0/bin/../mods/mobs_redo/api.lua:2504: in function <...de Mods/minetest 0-5-0/bin/../mods/mobs_redo/api.lua:2394>
2018-06-22 20:25:53: ERROR[Main]:   (tail call): ?
hkzorman commented 6 years ago

@BrunoMine Done. Test the most recent push.

Regarding the release, thanks for raising that question. I believe we should both release new versions. While I'm aware that for this branch there could be more bugs, I believe it is stable enough for now. Tell me when you are fully done and I will try also to merge my branch with master.

BrunoMine commented 6 years ago
2018-06-22 20:55:53: WARNING[Server]: Undeclared global variable "occupation_name" accessed at ...Mods/minetest 0-5-0/bin/../mods/advanced_npc/npc.lua:582
2018-06-22 20:55:53: ERROR[Main]: ServerError: AsyncErr: ServerThread::run Lua: Runtime error from mod 'sunos' in callback luaentity_Activate(): ...Mods/minetest 0-5-0/bin/../mods/advanced_npc/npc.lua:131: bad argument #1 to 'unpack' (table expected, got nil)
2018-06-22 20:55:53: ERROR[Main]: stack traceback:
2018-06-22 20:55:53: ERROR[Main]:   [C]: in function 'unpack'
2018-06-22 20:55:53: ERROR[Main]:   ...Mods/minetest 0-5-0/bin/../mods/advanced_npc/npc.lua:131: in function 'get_random_name'
2018-06-22 20:55:53: ERROR[Main]:   ...Mods/minetest 0-5-0/bin/../mods/advanced_npc/npc.lua:602: in function 'initialize'
2018-06-22 20:55:53: ERROR[Main]:   ...etest 0-5-0/bin/../mods/sociedades/sunos/npc/npc.lua:308: in function 'on_spawn'
2018-06-22 20:55:53: ERROR[Main]:   ...de Mods/minetest 0-5-0/bin/../mods/mobs_redo/api.lua:2504: in function <...de Mods/minetest 0-5-0/bin/../mods/mobs_redo/api.lua:2394>
2018-06-22 20:55:53: ERROR[Main]:   (tail call): ?
2018-06-22 20:55:53: ERROR[Main]:   [C]: in function 'add_entity'
2018-06-22 20:55:53: ERROR[Main]:   ...etest 0-5-0/bin/../mods/sociedades/sunos/npc/npc.lua:109: in function 'spawn'
2018-06-22 20:55:53: ERROR[Main]:   ...bin/../mods/sociedades/sunos/estruturas/casa/bau.lua:76: in function 'func_spawn'
2018-06-22 20:55:53: ERROR[Main]:   ...t 0-5-0/bin/../mods/sociedades/sunos/npc/spawner.lua:160: in function <...t 0-5-0/bin/../mods/sociedades/sunos/npc/spawner.lua:106>
hkzorman commented 6 years ago

Please check latest push, fad6531

By the way, I have forgotten to mention. I have added the ability to specify which search tags you want for choosing a NPC name when initializing it.

First you need to register a name:

npc.info.register_name("Name1", {"sociedades", "male"})
npc.info.register_name("Name2", {"sociedades", "unisex"})
npc.info.register_name("Name3", {"advanced_npc", "unisex"})

Then, you can pass tags to npc.initialize() in the npc_info argument:

npc.initialize(self, self.object:getpos(), true, nil, {name={tags={"sociedades"}}})

In the above example, only names with "sociedades" tag will be selected.

BrunoMine commented 6 years ago

ok, now this run without craches, but the tags not work. Names

-- Nomes de NPCs (derivados do esperanto)
sunos.var.npc_names = {
    male = {
        "Lumo", -- Luz
        "Racio", -- Razão
        "Honoro", -- Honra
        "Akurata", -- Pontual
        "Bonfama", -- de boa fama
        "Bonulo", -- Bom
        "Kora", -- Cordial
        "Negro", -- Negro
        "Obei", -- Obedecer
        "Obstini", -- Teimar
        "Poeto", -- Poeta
        "Trunko", -- Tronco
        "Flavo", -- Amarelo
    },
    female = {
        "Flava", -- Amarela
        "Hela", -- Luminosa
        "Brava", -- valente
        "Afable", -- Amavel
        "Bela", -- Bela
        "Danka", -- Grato
        "Eleganta", -- Elegante
        "Epopea", -- Épico
        "Negra", -- Negra
        "Nigra", -- Negra
        "Obeema", -- Obediente
    },
}
for _, name in ipairs(sunos.var.npc_names.male) do
    npc.info.register_name(name, {"male", "sunos"})
end
for _, name in ipairs(sunos.var.npc_names.female) do
    npc.info.register_name(name, {"female", "sunos"})
end

Initialize

if self.initialized == nil then
    npc.initialize(self, self.object:getpos(), true, nil, {name={tags={"sunos"}}})
    self.tamed = false
end

Results:

2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with occupation values
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Initializing NPC at (9012,9.5,4780)
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: NPC Inventory: {
    "default:iron_lump 7",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    "",
    ""
}
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with name "Anonymous", gender: male, is child: nil, texture: {
    "sunos_npc_male.png"
}
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Overriding NPC values using occupation '"sunos_npc_caseiro"' values
2018-06-22 22:46:35: [Server]: Result: "sunos_npc_male.png"
2018-06-22 22:46:35: [Server]: def.properties: nil
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with occupation values
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Initializing NPC at (9013,9.5,4781)
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with name "Anonymous", gender: female, is child: nil, texture: {
    "sunos_npc_female.png"
}
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Overriding NPC values using occupation '"sunos_npc_caseiro"' values
2018-06-22 22:46:35: [Server]: Result: "sunos_npc_female.png"
2018-06-22 22:46:35: [Server]: def.properties: nil
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with occupation values
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Initializing NPC at (9022,9.5,4779)
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with name "Anonymous", gender: female, is child: nil, texture: {
    "sunos_npc_male.png"
}
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Overriding NPC values using occupation '"sunos_npc_caseiro"' values
2018-06-22 22:46:35: [Server]: Result: "sunos_npc_female.png"
2018-06-22 22:46:35: [Server]: def.properties: nil
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with occupation values
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Initializing NPC at (9033,9.5,4783)
2018-06-22 22:46:35: [Server]: [advanced_npc] INFO: Successfully initialized NPC with name "Anonymous", gender: female, is child: nil, texture: {
    "sunos_npc_female.png"
}
hkzorman commented 6 years ago

Please try with latest push, I have tried in my local and the names you provided seem to be assigned correctly. However, the textures are not. I saw a male getting the female texture, not sure why.

Will try to check why.

BrunoMine commented 6 years ago

Very nice, the name tags works nice here. The branch has not been updated yet, but I'll do it now

BrunoMine commented 6 years ago

The last thing that needs to be resolved before we launch is this. Without resolving this, we will not be able to run with 0.4.16 version.

BrunoMine commented 6 years ago

Okay, this is okay for merge, my project woks fine.

hkzorman commented 6 years ago

Awesome, I guess you will test before doing any formal release, right?

BrunoMine commented 6 years ago

I have already done several tests so far, I believe it is 90% functional. After the launch, new problems may appear. But we need to launch to test. Launch your version first, so users can download the new version (clean_api) of advanced_api.

hkzorman commented 6 years ago

Merged at last. Release is here: https://github.com/hkzorman/advanced_npc/releases/tag/1.0.0-alpha-2

Please feel free to direct people to the release link. Also, please test!