Skip to content

Commit

Permalink
v2.1.11
Browse files Browse the repository at this point in the history 
Fixed a critical bug: our Megalo variable networking priority enum was wrong: there is no such value as "default." The value is no longer used; trying to compile it emits a unique error message; loading game variants which use it will correct them on load. Documentation updated.

The compiler is now capable of emitting "notice" messages.

The compiler emits notices for unused functions, and for functions used in only one place (which would decompile as blocks, not functions).

Renamed `unk_14_team` to `hud_target_object_team`. Documentation updated.

All Megalo keywords receive the start position (i.e. the start position of the initial keyword). This should slightly improve error logging.
  • Loading branch information
@DavidJCobb
DavidJCobb committed Mar 3, 2022
1 parent 51590b8 commit aa27fc8
Show file tree
Hide file tree
Showing 16 changed files with 299 additions and 116 deletions.
2 changes: 1 addition & 1 deletion native/src/ReachVariantTool/ReachVariantTool.rc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ IDI_ICON1 ICON DISCARDABLE "ReachVariantTool.ico"
// Edit these to control the embedded version information.
#define VER_MAJOR 2
#define VER_MINOR 1
#define VER_PATCH 10
#define VER_PATCH 11
#define VER_BUILD 0

// No need to edit anything below here. :)
Expand Down
23 changes: 20 additions & 3 deletions native/src/ReachVariantTool/docs/working - help docs/script/api/ns_unnamed.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,26 @@
<category id="unscoped-hud" />
<category id="unscoped-team" />
</member>
<member name="unk_14_team" type="team">
<blurb>Unknown, and potentially unused.</blurb>
<related name="unk_15_team" />
<member name="hud_target_object_team" type="team">
<blurb>This value is believed to refer to the owner team for <code>hud_target_object</code>.</blurb>
<description>
<p>
Community testing indicates that this value is one of several that can be used
with HUD widgets and object waypoints to display different information to each
player. When setting a widget or waypoint's text, you would pass this value
(or a member variable on it) as a <a href="script/api/format-string.html">format string</a>
parameter. Testing suggests that this refers to the owner team for <a href="hud_target_object.html">hud_target_object</a>'s owner team.
</p>
<p>
These values would be useful for UI code. Consider, for example, a situation
where you want to put a waypoint on an object, and have that object display the value of <code>&lt;self&gt;.team.number[0]</code>
as the waypoint's text. There wouldn't be any way to pass that unless you could
write something like <code>hud_target_object_team.number[0]</code> into the
format string.
</p>
</description>
<category id="unscoped-hud" />
<category id="unscoped-team" />
</member>
<member name="unk_15_team" type="team">
<blurb>Unknown, and potentially unused.</blurb>
Expand Down
18 changes: 7 additions & 11 deletions native/src/ReachVariantTool/docs/working - help docs/script/declare.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
<body>
<p>
Variable declarations allow you to specify a variable's networking priority and its
initial value. They aren't required &mdash; you can use a variable straightaway,
without declaring it &mdash; but because Megalo's default variable networking priority
can't survive host migrations, you'll probably want to use them.
</p>
<p>
Variable declarations look like this:
initial value. They look like this:
</p>
<pre>
declare SOME_VARIABLE
Expand All @@ -30,16 +25,17 @@
priority values are:
</p>
<dl>
<dt>default</dt>
<dd>The meaning of this value is unclear. Testing has established that a variable's
value will be lost after a host migration if its network priority was the default.</dd>
<dt>low</dt>
<dd>Testing has established that low-priority variables can survive a host migration.</dd>
<dd>Testing has established that low-priority variables can survive a host migration.
If you don't specify a networking priority for a variable, this is used as the
default.</dd>
<dt>high</dt>
<dd>Testing has established that high-priority variables can survive a host migration.
If you wish to display a player variable in the UI, it must be high-priority.</dd>
<dt>local</dt>
<dd>Variables with this network priority aren't sent over the network at all.</dd>
<dd>Variables with this network priority aren't sent over the network at all. This
is a good thing to use for temporary variables that are just used to do quick tasks,
like computing a number to be stored elsewhere.</dd>
</dl>
<p>
Declaring a variable multiple times generates a compiler warning if the declarations
Expand Down
4 changes: 2 additions & 2 deletions native/src/ReachVariantTool/docs/working - help docs/script/syntax.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@
<p>
In this particular Megalo dialect, you cannot define your own variables; there are a
fixed number available. However, you can <a href="alias">give names to variables</a>,
and <a href="declare">variable declarations</a> can (and should) be used to set a
variable's networking priority and initial value.
and <a href="declare">variable declarations</a> can be used to set a variable's
networking priority and initial value.
</p>

<h3>Properties</h3>
Expand Down
109 changes: 80 additions & 29 deletions native/src/ReachVariantTool/game_variants/components/megalo/compiler/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,36 @@ namespace Megalo {
block->compile(compiler);
if (!block->trigger) // empty blocks get skipped
continue;
if (block->type == Block::Type::function) // don't mis-compile function definitions as function calls
if (block->type == Block::Type::function) {
//
// We don't want to mis-compile function definitions as function calls. Additionally, we should
// emit compile-time warnings or notices based on function usage.
//
if (block->caller_count < 2) {
Compiler::pos start;
start.line = block->line;
start.offset = block->range.start;
start.last_newline = block->range.start - block->col;
if (block->caller_count == 0) {
compiler.raise_notice(
start,
QString("User-defined function \"%1\" isn't called from anywhere. Is this intentional?")
.arg(block->name)
);
} else if (block->caller_count == 1) {
compiler.raise_notice(
start,
QString("User-defined function \"%1\" is only called from one place. When decompiling this gametype, it will not appear as a function; a function that is called from only one place is indistinguishable from a nested block.")
.arg(block->name)
);

}
}
//
// Don't compile a call:
//
continue;
}
auto* block_trigger = block->tr_wrap ? block->tr_wrap : block->trigger; // needed for the on:for workaround
if (block_trigger != this->trigger) {
//
Expand Down Expand Up @@ -564,6 +592,14 @@ namespace Megalo {
}
}
}

void Block::for_each_function(std::function<void(Block*)> functor) {
if (this->type == Type::function)
(functor)(this);
for (auto* child : this->items)
if (auto* nested = dynamic_cast<Block*>(child))
nested->for_each_function(functor);
}
#pragma endregion
//
Statement::~Statement() {
Expand Down Expand Up @@ -638,7 +674,7 @@ namespace Megalo {
return *this;
}
}
//

Compiler::~Compiler() {
for (auto trigger : this->results.triggers)
delete trigger;
Expand Down Expand Up @@ -1025,12 +1061,14 @@ namespace Megalo {
//
Compiler::log_checkpoint Compiler::create_log_checkpoint() {
log_checkpoint point;
point.notices = this->notices.size();
point.warnings = this->warnings.size();
point.errors = this->errors.size();
point.errors = this->errors.size();
point.fatal_errors = this->fatal_errors.size();
return point;
}
void Compiler::revert_to_log_checkpoint(Compiler::log_checkpoint point) {
this->notices.resize(point.notices);
this->warnings.resize(point.warnings);
this->errors.resize(point.errors);
this->fatal_errors.resize(point.fatal_errors);
Expand Down Expand Up @@ -1072,6 +1110,15 @@ namespace Megalo {
void Compiler::raise_warning(const QString& text) {
this->warnings.emplace_back(text, this->state);
}
void Compiler::raise_warning(const Compiler::pos& pos, const QString& text) {
this->warnings.emplace_back(text, pos);
}
void Compiler::raise_notice(const QString& text) {
this->notices.emplace_back(text, this->state);
}
void Compiler::raise_notice(const Compiler::pos& pos, const QString& text) {
this->notices.emplace_back(text, pos);
}
void Compiler::validate_format_string_tokens(const QString& text) {
auto issues = check_format_string(text);
if (issues.empty())
Expand Down Expand Up @@ -1340,6 +1387,7 @@ namespace Megalo {
Script::VariableReference* lhs = nullptr;
Script::VariableReference* rhs = nullptr;
//
this->skip_whitespace();
auto prior = this->backup_stream_state();
//
#pragma region Parsing
Expand All @@ -1353,7 +1401,7 @@ namespace Megalo {
lhs = new Script::VariableReference(integer);
} else if (lhs_type == statement_side::word) {
if (auto handler = Compiler::__get_handler_for_keyword(word)) {
((*this).*(handler))();
((*this).*(handler))(prior);
return;
}
if (word == "and" || word == "or" || word == "not" || word == "then") {
Expand Down Expand Up @@ -2167,6 +2215,11 @@ namespace Megalo {
statement->set_start(call_start);
statement->set_end(this->state);
this->block->insert_item(statement);
//
if (func->content) {
++func->content->caller_count;
}
//
return nullptr;
}
//
Expand Down Expand Up @@ -2573,9 +2626,7 @@ namespace Megalo {
}
//
#pragma region keyword handlers
void Compiler::_handleKeyword_Alias() {
auto start = this->backup_stream_state();
//
void Compiler::_handleKeyword_Alias(const pos start) {
auto name = this->extract_word();
if (name.isEmpty()) {
this->raise_fatal("An alias declaration must supply a name.");
Expand Down Expand Up @@ -2622,7 +2673,7 @@ namespace Megalo {
if (!item->invalid) // aliases can also run into non-fatal errors
this->aliases_in_scope.push_back(item);
}
void Compiler::_handleKeyword_Alt() {
void Compiler::_handleKeyword_Alt(const pos start) {
if (this->block->type != Script::Block::Type::if_block && this->block->type != Script::Block::Type::altif_block) {
auto prev = this->block->item(-1);
auto p_bl = dynamic_cast<Script::Block*>(prev);
Expand All @@ -2639,7 +2690,7 @@ namespace Megalo {
}
auto item = new Script::Block;
item->type = Script::Block::Type::alt_block;
item->set_start(this->state);
item->set_start(start);
this->block->insert_item(item);
this->_openBlock(item);
{
Expand All @@ -2649,7 +2700,7 @@ namespace Megalo {
item->make_alt_of(*block);
}
}
void Compiler::_handleKeyword_AltIf() {
void Compiler::_handleKeyword_AltIf(const pos start) {
if (this->block->type != Script::Block::Type::if_block && this->block->type != Script::Block::Type::altif_block) {
auto prev = this->block->item(-1);
auto p_bl = dynamic_cast<Script::Block*>(prev);
Expand All @@ -2666,7 +2717,7 @@ namespace Megalo {
}
auto item = new Script::Block;
item->type = Script::Block::Type::altif_block;
item->set_start(this->state);
item->set_start(start);
this->block->insert_item(item);
this->_openBlock(item);
{
Expand Down Expand Up @@ -2760,8 +2811,7 @@ namespace Megalo {
}
}
}
void Compiler::_handleKeyword_Declare() {
auto start = this->backup_stream_state();
void Compiler::_handleKeyword_Declare(const pos start) {
using net_t = Megalo::variable_network_priority;
//
// declare [word]
Expand All @@ -2781,7 +2831,7 @@ namespace Megalo {
}
//
bool has_network = false;
net_t network = net_t::normal;
net_t network = net_t::low;
#pragma region Parsing and extracting all relevant tokens
auto after_name = this->backup_stream_state();
auto word = this->extract_word();
Expand Down Expand Up @@ -2823,7 +2873,8 @@ namespace Megalo {
return;
}
if (word.compare("default", Qt::CaseInsensitive) == 0) {
network = net_t::normal;
this->raise_error("Value \"default\" is not a valid network priority. Older versions of ReachVariantTool (2.1.10 and older) incorrectly treated it as one. You should probably use \"low\" instead.");
network = net_t::low;
} else if (word.compare("low", Qt::CaseInsensitive) == 0) {
network = net_t::low;
} else if (word.compare("high", Qt::CaseInsensitive) == 0) {
Expand Down Expand Up @@ -2934,20 +2985,20 @@ namespace Megalo {
delete initial;
}

void Compiler::_handleKeyword_Do() {
void Compiler::_handleKeyword_Do(const pos start) {
auto item = new Script::Block;
item->type = Script::Block::Type::basic;
item->set_start(this->state);
item->set_start(start);
item->event = this->next_event;
this->next_event = Script::Block::Event::none;
this->block->insert_item(item);
this->_openBlock(item);
}
void Compiler::_handleKeyword_End() {
void Compiler::_handleKeyword_End(const pos start) {
if (!this->_closeCurrentBlock())
this->raise_fatal("Unexpected \"end\".");
}
void Compiler::_handleKeyword_Enum() {
void Compiler::_handleKeyword_Enum(const pos start) {
auto name = this->extract_word();
{ // Validate the name.
if (name.isEmpty()) {
Expand Down Expand Up @@ -3083,19 +3134,17 @@ namespace Megalo {
//
this->enums_in_scope.emplace_back(def.release(), this->block);
}
void Compiler::_handleKeyword_If() {
void Compiler::_handleKeyword_If(const pos start) {
auto item = new Script::Block;
item->type = Script::Block::Type::if_block;
item->set_start(this->state);
item->set_start(start);
item->event = this->next_event;
this->next_event = Script::Block::Event::none;
this->block->insert_item(item);
this->_openBlock(item);
this->_parseBlockConditions();
}
void Compiler::_handleKeyword_For() {
auto start = this->backup_stream_state();
//
void Compiler::_handleKeyword_For(const pos start) {
if (!this->extract_word("each")) {
this->raise_fatal("The \"for\" keyword must be followed by \"each\".");
return;
Expand Down Expand Up @@ -3168,9 +3217,7 @@ namespace Megalo {
this->block->insert_item(item);
this->_openBlock(item);
}
void Compiler::_handleKeyword_Function() {
auto start = this->backup_stream_state();
//
void Compiler::_handleKeyword_Function(const pos start) {
auto name = this->extract_word();
if (name.isEmpty()) {
this->raise_fatal("A function must have a name.");
Expand Down Expand Up @@ -3261,10 +3308,14 @@ namespace Megalo {
// trigger. So, we'll create it early.
//
item->trigger = new Trigger;
this->functions_in_scope.push_back(Script::UserDefinedFunction(name, this->results.triggers.size(), item)); // remember this function and its index
this->functions_in_scope.push_back(Script::UserDefinedFunction( // remember this function and its index
name,
this->results.triggers.size(),
*item
));
this->results.triggers.push_back(item->trigger);
}
void Compiler::_handleKeyword_On() {
void Compiler::_handleKeyword_On(const pos start) {
if (this->block != this->root) {
this->raise_error("Only top-level (non-nested) blocks can be event handlers.");
if (!this->skip_to(':'))
Expand Down
Loading

0 comments on commit aa27fc8

Please sign in to comment.