Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make ammo effect able to run EoC #75291

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

GuardianDll
Copy link
Member

@GuardianDll GuardianDll commented Jul 28, 2024

Summary

None

Purpose of change

I don't like how new damage types are used only because they can run EoC on attack

Describe the solution

Add ammo_effect field, that runs EoC

Testing

That's the problem, i use next EoC to test thing

{
  "type": "effect_on_condition",
  "id": "EOC_DEBUG_MESSAGE",
  "effect": [
    { "u_message": "DEBUG MESSAGE U.  Alpha: <u_name>, Beta: <npc_name>; <context_val:proj_damage>" },
    { "npc_message": "DEBUG MESSAGE BPC.  Alpha: <u_name>, Beta: <npc_name>; <context_val:proj_damage>" },
    { "message": "DEBUG MESSAGE GLOBAL.  Alpha: <u_name>, Beta: <npc_name>; <context_val:proj_damage>" }
  ]
},

but for hellish reason result looks like this:
image
So u_message is not run, despite u_name being assigned correctly. Any another u_ effects do not apply. I have no idea why it happens, my code uses the same tools as any another eoc-running function. I suspect npc_ do not work correctly either

Secondly, because of projectile ability to miss the target, EoC can be built either with beta talker as victim, or without beta talker whatsoever. It complicates the thing, because user is not able to use npc_ functions, because, well, we don't have an effect to check is beta talker presented (and i don't even know can such effect exist).


UPD:

Andrei helped to solve both issues, so it's good now (theoretically)

Additional context

@andrei8l sorry to bother you, but may i ask for some help?

@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [C++] Changes (can be) made in C++. Previously named `Code` [Markdown] Markdown issues and PRs Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jul 28, 2024
@andrei8l
Copy link
Contributor

@andrei8l sorry to bother you, but may i ask for some help?

I'm assuming you're asking why only one message is displayed.
First issue is that talker implementations were split into talker_X and talker_X_const without const correctness throughout the code. This should "fix" u_message:

diff --git a/src/npctalk.cpp b/src/npctalk.cpp
index 94f785f0dc..cf06a46029 100644
--- a/src/npctalk.cpp
+++ b/src/npctalk.cpp
@@ -4330,11 +4330,11 @@ talk_effect_fun_t::func f_message( const JsonObject &jo, std::string_view member
     return [snip_id, message, outdoor_only, sound, snippet, same_snippet, type_string,
                      popup_msg, popup_w_interrupt_query_msg, interrupt_type, global, is_npc]
     ( dialogue const & d ) {
-        Character *target;
+        Character const *target;
         if( global ) {
             target = &get_player_character();
         } else {
-            target = d.actor( is_npc )->get_character();
+            target = static_cast<talker const*>( d.actor( is_npc ) )->get_character();
         }
         if( !target || target->is_npc() ) {
             return;

Second issue is that npc_message only works for Characters (not monsters) and is explicitly disabled for... NPCs. I have no idea why.

Cataclysm-DDA/src/npctalk.cpp

Lines 4339 to 4341 in bfeb1ff

if( !target || target->is_npc() ) {
return;
}

because, well, we don't have an effect to check is beta talker presented (and i don't even know can such effect exist).

If you meant a condition, then try this
diff --git a/src/condition.cpp b/src/condition.cpp
index 41e06c54f8..8a5321553f 100644
--- a/src/condition.cpp
+++ b/src/condition.cpp
@@ -1314,6 +1314,13 @@ conditional_t::func f_player_see( bool is_npc )
     };
 }
 
+conditional_t::func f_has_beta()
+{
+    return []( dialogue const & d ) {
+        return d.has_beta;
+    };
+}
+
 conditional_t::func f_no_assigned_mission()
 {
     return []( dialogue const & d ) {
@@ -2554,6 +2561,7 @@ parsers_simple = {
     {"u_is_furniture", "npc_is_furniture", &conditional_fun::f_is_furniture },
     {"has_ammo", &conditional_fun::f_has_ammo },
     {"player_see_u", "player_see_npc", &conditional_fun::f_player_see },
+    {"has_beta", &conditional_fun::f_has_beta },
 };
 
 conditional_t::conditional_t( const JsonObject &jo )

@GuardianDll
Copy link
Member Author

GuardianDll commented Jul 28, 2024

I'm assuming you're asking why only one message is displayed

I didn't expect the issue would be in f_message. I also tried to use u_pain() to test, and also got no effect - does it mean it also has similar issue? how many functions has such issue overall?

If you meant a condition, then try this

Yes, but would it even work? In order to check beta exists you need to reach beta, which is nullptr, no?
I'll test it and confirm

UPD: no, it works as you expect it, and doesn't cause any errors. Thank you!
I still have question about u_pain(), does all u_ functions require rewriting to make them consistent?

@github-actions github-actions bot added NPC / Factions NPCs, AI, Speech, Factions, Ownership EOC: Effects On Condition Anything concerning Effects On Condition labels Jul 28, 2024
@GuardianDll GuardianDll marked this pull request as ready for review July 28, 2024 13:38
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 29, 2024
@dseguin dseguin merged commit 24395f1 into CleverRaven:master Jul 29, 2024
36 of 39 checks passed
@GuardianDll GuardianDll deleted the eoc_ammo_effect branch July 29, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. EOC: Effects On Condition Anything concerning Effects On Condition Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants