Skip to content

Full latcom rewrite#783

Merged
heinermann merged 8 commits intobwapi:developfrom
N00byEdge:develop
Apr 3, 2018
Merged

Full latcom rewrite#783
heinermann merged 8 commits intobwapi:developfrom
N00byEdge:develop

Conversation

@N00byEdge
Copy link
Member

@N00byEdge N00byEdge commented Mar 26, 2018

Latcom has always been somewhat messy with BWAPI. We've seen issues related to negative minerals, crazy wrong supply etc. My goal with this rewrite was of course not to make latcom perfect (way too hard of a problem) but I wanted to make sure none of these bugs were expected. Old latcom code was also a lot harder to maintain and just wasn't built around that there could be a turn size that isn't 1.

New latcom code handles any turn size or latency. It is based on the timings collected by jaj22.

If there are any questions or requests, I should be able to look at them shortly.

Fixes, #781, #652, #613, etc. I haven't gone back further but there are probably more issues related to latcom.

@bwapi-build
Copy link

Build bwapi 4.2.0.96-qeehtdry completed (commit 3f09029d47 by @N00byEdge)

@N00byEdge
Copy link
Member Author

We should hold off on this PR for a bit, I still have to solve one thing for the client.

@N00byEdge
Copy link
Member Author

That should be it, have at it.

@bwapi-build
Copy link

void execute(bool isCurrentFrame);

template<typename Buf>
void insertIntoCommandBuffer(Buf &buf) && {
Copy link
Member

Choose a reason for hiding this comment

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

What is this syntax?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the buffer is vector<vector<Command>>, and CommandTemp<...> is what makes up the type Command, this lets the caller use any vector<vector<T>> buffer, just to avoid any circular dependenices.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh wait, this is why I made Command an alias instead of its own class. Guess I could change that to be based on the template parameters now then.

Copy link
Member Author

@N00byEdge N00byEdge Mar 27, 2018

Choose a reason for hiding this comment

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

You know what? While I made that edit I just realized that you meant the && part. It means that the function will only be used when the CommandTemp<> is an rvalue, which is what you want since there is a std::move(*this) inside of it when it moves itself into the buffer.

Copy link
Member Author

Choose a reason for hiding this comment

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

See here for the original proposal and this for a explanation. This is how you make *this become an rvalue.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can see how this is called here

flags.fill(false);

// Clear the latency buffer
for(unsigned int j = 0; j < this->commandBuffer.size(); ++j)
Copy link
Member

Choose a reason for hiding this comment

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

Is there code that clears the latency compensation buffer between games?

Copy link
Member Author

Choose a reason for hiding this comment

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

As you can see at
https://github.com/bwapi/bwapi/pull/783/files#diff-6464d0d2ecc3b9271d1d8112b1a20331L319,
the vector now contains value types. The clear() at https://github.com/bwapi/bwapi/pull/783/files#diff-5b9870b36b881f8a0e19b037bcb74c17L330 then does everything. There are no more pointers in the latcom code.

@heinermann
Copy link
Member

Can you check if this works with and without the command optimizer (highest level)? IIRC they were incompatible before, does this change that at all?

@N00byEdge
Copy link
Member Author

What would there be to test without the command optimizer? Just that it works at all or some specific behavior?

@bwapi-build
Copy link

@bwapi-build
Copy link

@bwapi-build
Copy link

@N00byEdge
Copy link
Member Author

The testing I've done so far includes CommandOptimizer set to 2 on a module bot and none on a client one.

Sorry for these, being delayed, morph supply was not incuded in the "BW
order timings" document. I have requested edit access to add these too.
@bwapi-build
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants