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.

Sign up for GitHub

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

Jump to bottom

More message filters #5605

Merged
merged 6 commits into from
Oct 18, 2023
Merged

More message filters #5605

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9586f21
Add team filters to messages
naomimyselfandi Sep 16, 2023
04a5778
Allow filtering builtin messages by nearby ships
naomimyselfandi Sep 16, 2023
f13a5b3
Allow specifying tiebreakers between builtin messages
naomimyselfandi Sep 16, 2023
0811c70
Allow (easily) requiring an exact mood match
naomimyselfandi Sep 16, 2023
15db3b1
Respond to PR feedback
naomimyselfandi Sep 24, 2023
2bcedc3
Respond to PR feedback
naomimyselfandi Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 114 additions & 45 deletions code/mission/missionmessage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "iff_defs/iff_defs.h"
#include "io/timer.h"
#include "localization/localize.h"
#include "math/vecmat.h"
#include "mission/missiontraining.h"
#include "mission/missiongoals.h"
#include "mod_table/mod_table.h"
Expand All @@ -44,6 +45,7 @@
bool Allow_generic_backup_messages = false;
float Command_announces_enemy_arrival_chance = 0.25;

#define DEFAULT_MOOD 0
SCP_vector<SCP_string> Builtin_moods;
int Current_mission_mood;

Expand All @@ -61,6 +63,16 @@ builtin_message Builtin_messages[] = {
#undef X
};

#define BUILTIN_BOOST_LEVEL_ONE 1
#define BUILTIN_BOOST_LEVEL_TWO 2
#define BUILTIN_MATCHES_TYPE 4
#define BUILTIN_MATCHES_FILTER 8
#define BUILTIN_MATCHES_MOOD 16
#define BUILTIN_MATCHES_SPECIES 32
#define BUILTIN_MATCHES_PERSONA 64
TRBlount marked this conversation as resolved.
Show resolved Hide resolved

#define BUILTIN_BOOST_LEVEL_THREE (BUILTIN_BOOST_LEVEL_ONE | BUILTIN_BOOST_LEVEL_TWO)

int get_builtin_message_type(const char* name) {
for (int i = 0; i < MAX_BUILTIN_MESSAGE_TYPES; i++) {
if (!stricmp(Builtin_messages[i].name, name)) {
Expand Down Expand Up @@ -338,6 +350,7 @@ int add_wave( const char *wave_name )
void message_filter_clear(MessageFilter& filter) {
filter.species_bitfield = 0;
filter.type_bitfield = 0;
filter.team_bitfield = 0;
}

void message_filter_parse(MessageFilter& filter) {
Expand Down Expand Up @@ -374,13 +387,24 @@ void message_filter_parse(MessageFilter& filter) {
stuff_string(buf, F_NAME);
int type = ship_type_name_lookup(buf.c_str());
if (type < 0) {
Warning(LOCATION, "Unknown type %s in messages.tbl", buf.c_str());
Warning(LOCATION, "Unknown ship type %s in messages.tbl", buf.c_str());
} else if (type >= 32) {
Warning(LOCATION, "Type %s is index 32 or higher and therefore cannot be used in a message filter", buf.c_str());
} else {
filter.type_bitfield |= (1 << type);
}
}
while (optional_string("+Team:")) {
stuff_string(buf, F_NAME);
int team = iff_lookup(buf.c_str());
if (team < 0) {
Warning(LOCATION, "Unknown team %s in messages.tbl", buf.c_str());
} else if (team >= 32) {
Warning(LOCATION, "Team %s is index 32 or higher and therefore cannot be used in a message filter", buf.c_str());
} else {
filter.team_bitfield |= (1 << team);
}
}
}

void handle_legacy_backup_message(MissionMessage& msg, SCP_string wing_name) {
Expand All @@ -403,6 +427,16 @@ void handle_legacy_backup_message(MissionMessage& msg, SCP_string wing_name) {
strcpy(msg.name, backup);
}

int lookup_mood(SCP_string& name) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be sticking to making things like this name parameter const as often as we can

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhere where my C++ is a little lacking. SCP_string const & means "a reference to an SCP_string which we promise not to change" and potentially allows for compiler optimizations, right? What about declaring the function itself const? (To be clear, I'm asking from an educational perspective ^^ )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, basically.
SCP_string const & gives you a reference to a string you cannot change. The compiler will stop you from any things that would change the string, so not only allows it for more optimization, but also is simply a safeguard for you as the developer to not do things you shouldn't.
Declaring the function const in this context is not possible. You could declare the return type const (const int lookup_mood), but declaring the function const (int lookup_mood(...) const {) is only possible if it's a member function of a class (which it isn't, as a global function), and will basically mean "disallow any modification of member variables", which allows you to call the function on an object of that class which is const itself.

for (auto i = Builtin_moods.begin(); i != Builtin_moods.end(); ++i) {
if (*i == name) {
TRBlount marked this conversation as resolved.
Show resolved Hide resolved
return (int) std::distance(Builtin_moods.begin(), i);
}
}
Warning(LOCATION, "Message.tbl has an entry for mood type %s, but this mood is not in the #Moods section of the table.", name.c_str());
return -1;
}

// parses an individual message
void message_parse(MessageFormat format) {
MissionMessage msg;
Expand Down Expand Up @@ -486,45 +520,31 @@ void message_parse(MessageFormat format) {
}
}

bool require_exact_mood_match;
if (optional_string("$Mood:")) {
SCP_string buf;
bool found = false;

stuff_string(buf, F_NAME);
for (SCP_vector<SCP_string>::iterator iter = Builtin_moods.begin(); iter != Builtin_moods.end(); ++iter) {
if (*iter == buf) {
msg.mood = (int)std::distance(Builtin_moods.begin(), iter);
found = true;
break;
}
}

if (!found) {
// found a mood, but it's not in the list of moods at the start of the table
Warning(LOCATION, "Message.tbl has an entry for mood type %s, but this mood is not in the #Moods section of the table.", buf.c_str());
}
}
else {
msg.mood = 0;
SCP_string buf;
stuff_string(buf, F_NAME);
msg.mood = lookup_mood(buf);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a mood isn't found, msg.mood will now be set to -1 instead of 0. Which in turn means that erroneous moods for messages caused the messages to be boosted when the mood was actually 0.
While I doubt that that has been taken advantage off in an existing mod, it should probably at least get a comment here acknowledging that change, in case anyone comes looking in the future.

Copy link
Contributor Author

@naomimyselfandi naomimyselfandi Sep 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it? Looking at the code on master, I'm not sure that msg.mood is actually initialized on a missing mood. Isn't that UB?
e: following up on Discord

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, turns out I was confusing things, and the default-initialization implicitly causing a zero-initialization is only correct for variables with a static lifetime

require_exact_mood_match = optional_string("+Require exact match");
} else {
msg.mood = DEFAULT_MOOD;
require_exact_mood_match = false;
}

if (optional_string("$Exclude Mood:")) {
SCP_vector<SCP_string> buff;
bool found = false;

stuff_string_list(buff);
for (SCP_vector<SCP_string>::iterator parsed_moods = buff.begin(); parsed_moods != buff.end(); ++parsed_moods) {
for (SCP_vector<SCP_string>::iterator iter = Builtin_moods.begin(); iter != Builtin_moods.end(); ++iter) {
if (!stricmp(iter->c_str(), parsed_moods->c_str())) {
msg.excluded_moods.push_back((int)std::distance(Builtin_moods.begin(), iter));
found = true;
break;
}
if (require_exact_mood_match) {
for (auto i = Builtin_moods.begin(); i != Builtin_moods.end(); ++i) {
int mood = (int) std::distance(Builtin_moods.begin(), i);
if (mood != msg.mood) {
msg.excluded_moods.push_back(mood);
}

if (!found) {
// found a mood, but it's not in the list of moods at the start of the table
Warning(LOCATION, "Message.tbl has an entry for exclude mood type %s, but this mood is not in the #Moods section of the table.", parsed_moods->c_str());
}
} else if (optional_string("$Exclude Mood:")) {
SCP_vector<SCP_string> buf;
stuff_string_list(buf);
for (auto i = buf.begin(); i != buf.end(); ++i) {
int mood = lookup_mood(*i);
if (mood >= 0) {
msg.excluded_moods.push_back(mood);
}
}
}
Expand All @@ -541,6 +561,26 @@ void message_parse(MessageFormat format) {
message_filter_clear(msg.subject_filter);
}

msg.outer_filter_radius = -1;
if (optional_string("$Filter by other ship:")) {
if (optional_string("+Within range of sender:")) {
stuff_int(&msg.outer_filter_radius);
}
message_filter_parse(msg.outer_filter);
} else {
message_filter_clear(msg.outer_filter);
}

if (optional_string("$Prefer this message very highly")) {
msg.boost_level = BUILTIN_BOOST_LEVEL_THREE;
} else if (optional_string("$Prefer this message highly")) {
msg.boost_level = BUILTIN_BOOST_LEVEL_TWO;
} else if (optional_string("$Prefer this message")) {
msg.boost_level = BUILTIN_BOOST_LEVEL_ONE;
} else {
msg.boost_level = 0;
}

if (format == MessageFormat::TABLED) {
if (!stricmp(msg.name, "Beta Arrived")) {
handle_legacy_backup_message(msg, "Beta");
Expand All @@ -550,7 +590,7 @@ void message_parse(MessageFormat format) {
handle_legacy_backup_message(msg, "Delta");
} else if (!stricmp(msg.name, "Epsilon Arrived")) {
handle_legacy_backup_message(msg, "Epsilon");
} if (get_builtin_message_type(msg.name) == MESSAGE_NONE) {
} else if (get_builtin_message_type(msg.name) == MESSAGE_NONE) {
Warning(LOCATION, "Unknown builtin message type %s in messages.tbl", msg.name);
}
}
Expand Down Expand Up @@ -2017,7 +2057,8 @@ bool has_filters(MessageFilter& filter) {
|| !filter.class_name.empty()
|| !filter.wing_name.empty()
|| filter.species_bitfield
|| filter.type_bitfield;
|| filter.type_bitfield
|| filter.team_bitfield;
}

bool filter_matches(SCP_string value, SCP_vector<SCP_string>& filter) {
Expand Down Expand Up @@ -2045,10 +2086,30 @@ bool filters_match(MessageFilter& filter, ship* it) {
&& filter_matches(hud_get_ship_class(it), filter.class_name)
&& filter_matches(wing_name, filter.wing_name)
&& filter_matches(Ship_info[it->ship_info_index].species, filter.species_bitfield)
&& filter_matches(Ship_info[it->ship_info_index].class_type, filter.type_bitfield);
&& filter_matches(Ship_info[it->ship_info_index].class_type, filter.type_bitfield)
&& filter_matches(it->team, filter.team_bitfield);
}
}

bool outer_filters_match(MessageFilter& filter, int range, ship* sender) {
for (auto i: list_range(&Ship_obj_list)) {
auto obj = &Objects[i->objnum];
if (obj->flags[Object::Object_Flags::Should_be_dead]) {
continue;
}
if ((range >= 0) && (vm_vec_dist(&obj->pos, &Objects[sender->objnum].pos) > range)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>= ?
I mean, I guess an uninitialized range is set to -1, but is there ever a case where range == 0 is sensible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the sender itself is ignored (if (sender->objnum == i->objnum) { continue; }), the only way a range of 0 could ever be satisfied is with non-colliding objects in exactly the same position. I don't think it matters in practice, but > 0 is probably clearer, so I'm happy to make that change.

continue;
}
if (sender->objnum == i->objnum) {
continue;
}
if (filters_match(filter, &Ships[obj->instance])) {
return true;
}
}
return false;
}

bool excludes_current_mood(int message) {
for (SCP_vector<int>::iterator iter = Messages[message].excluded_moods.begin(); iter != Messages[message].excluded_moods.end(); ++iter) {
if (*iter == Current_mission_mood) {
Expand All @@ -2059,12 +2120,6 @@ bool excludes_current_mood(int message) {
}

int get_builtin_message_inner(int type, int persona, ship* sender, ship* subject, bool require_exact_persona_match) {
static const int BUILTIN_MATCHES_TYPE = 0;
static const int BUILTIN_MATCHES_FILTER = 1;
static const int BUILTIN_MATCHES_MOOD = 2;
static const int BUILTIN_MATCHES_SPECIES = 4;
static const int BUILTIN_MATCHES_PERSONA = 8;

const char* name = Builtin_messages[type].name;
SCP_vector<int> matching_builtins;
int match_level, best_match_level = 0;
Expand All @@ -2084,6 +2139,9 @@ int get_builtin_message_inner(int type, int persona, ship* sender, ship* subject
match_level = BUILTIN_MATCHES_TYPE;
}

// Apply "Prefer this message" flags
match_level |= Messages[i].boost_level;

if (Current_mission_mood == Messages[i].mood) {
// Boost messages that match the current mood
match_level |= BUILTIN_MATCHES_MOOD;
Expand Down Expand Up @@ -2120,6 +2178,17 @@ int get_builtin_message_inner(int type, int persona, ship* sender, ship* subject
}
}

// Ditto with outer filters, although they're more complicated
if (has_filters(Messages[i].outer_filter)) {
if (outer_filters_match(Messages[i].outer_filter, Messages[i].outer_filter_radius, sender)) {
// Boost messages that have at least one filter
match_level |= BUILTIN_MATCHES_FILTER;
} else {
// Ignore messages where any filter doesn't match
continue;
}
}

if (match_level == best_match_level) {
matching_builtins.push_back(i);
} else if (match_level > best_match_level) {
Expand Down
4 changes: 4 additions & 0 deletions code/mission/missionmessage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ typedef struct MessageFilter {
SCP_vector<SCP_string> wing_name;
int species_bitfield;
int type_bitfield;
int team_bitfield;
} MessageFilter;

typedef struct MissionMessage {
Expand All @@ -160,6 +161,9 @@ typedef struct MissionMessage {

MessageFilter sender_filter;
MessageFilter subject_filter;
MessageFilter outer_filter;
int outer_filter_radius;
int boost_level;

// unions for avi/wave information. Because of issues with Fred, we are using
// the union to specify either the index into the avi or wave arrays above,
Expand Down
Loading