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

slight improvement of autopilot #10

Closed
wants to merge 5 commits into from

Conversation

the-zorro
Copy link

  • undock support
  • stoping script on arriving to station

- undock support
- stoping script on arriving to station
@Viir
Copy link
Collaborator

Viir commented Dec 29, 2016

Thanks for the PR, feel free to split it into multiple Commits/PRs as you see fit.

I see some issues with merging commit f68bfea into master:

  • Visual Studio tells me the two versions of the file have different encodings as you can see the screenshot at http://i.imgur.com/olXQFJy.png Can we keep UTF8 encoding?
  • I would prefer to keep around the simplest possible auto pilot as an example for other people (Hence the "simple" in the name). Since your version takes into account more information and contains more branching I would prefer to offer it to the user in another script with a different file name.

Host.Log("Arrived, stopping here");
goto loop;
} else {
Undock();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not understand why the control flow for Undock has been implemented this way:
Do we want to call Undock multiple times? If not, what is the reason that the call to Undock is contained in the loop block?


while(fly)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the fly variable? Can this not be simplified by using the condition used below to assign to fly as the looping condition?

@the-zorro
Copy link
Author

the-zorro commented Dec 29, 2016 via email

@Viir
Copy link
Collaborator

Viir commented Dec 29, 2016

I'm using IntelliJ Rider as my
IDE and it doesn't assume some assemblies are referenced by default. Do you
mind if my changes will include some of those assemblies (System.Runtime,
System.IO) into project files?

It depends on what is in those assemblies and what the benefit of including those would be. Where would other users get these assemblies from?

@the-zorro
Copy link
Author

I added second commit. I removed Undock method entirely and just placed undock code before the main loop. Script stopping is done by break instead of variable. "simple" version is reverted, mine is called Travel.zorro.cs

Viir
Viir previously approved these changes Dec 30, 2016
@Viir Viir dismissed their stale review December 30, 2016 12:22

Made a mistake, did not mean to approve all changes.

@Viir Viir self-requested a review December 30, 2016 12:24
Copy link
Collaborator

@Viir Viir left a comment

Choose a reason for hiding this comment

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

When I try to build commit 0d9bf3b in Visual Studio 2015 it gives me those two compile errors:

Severity	Code	Description	Project	File	Line	Suppression State
Error	CS1703	Multiple assemblies with equivalent identity have been imported: 'C:\WINDOWS\Microsoft.Net\assembly\GAC_MSIL\System.IO\v4.0_4.0.0.0__b03f5f7f11d50a3a\System.IO.dll' and 'C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6\Facades\System.IO.dll'. Remove one of the duplicate references.	Sanderling	K:\Source\Repos\Sanderling\src\Sanderling\Sanderling\CSC	1	Active
Error	CS1703	Multiple assemblies with equivalent identity have been imported: 'C:\WINDOWS\Microsoft.Net\assembly\GAC_MSIL\System.Runtime\v4.0_4.0.0.0__b03f5f7f11d50a3a\System.Runtime.dll' and 'C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6\Facades\System.Runtime.dll'. Remove one of the duplicate references.	Sanderling	K:\Source\Repos\Sanderling\src\Sanderling\Sanderling\CSC	1	Active

Thus, it seems like the change breaks the build for users with Visual Studio 2015 and therefore can not be integrated into master.

I will look into setting up CI build to provide feedback for such problems faster in the future.

@the-zorro
Copy link
Author

Interesting. Is there way to make it so that it will satisfy both tools?
The thing is not actually in Rider itself. Rider uses M$ build to build solutions. I'll check if the solution can be built by pure M$ build later today.

@Viir
Copy link
Collaborator

Viir commented Dec 30, 2016

Is there way to make it so that it will satisfy both tools?

I don't know.

@the-zorro
Copy link
Author

After dozen tries it seems fixed by installing one of M$ development packs. I think it was 4.6.1. Now both m$build and my IDE can compile unmodified projects.

@Viir
Copy link
Collaborator

Viir commented Jan 15, 2017

I see that you added a commit. Since commit #0d9bf3b is still in this PR, I assume the PR is not complete yet.

@the-zorro
Copy link
Author

It's not complete. I found one thing I'd like to change in the autopilot script, but that other commits you see are probably from another branch. In that branch I've modified mining script a lot, but I'm not sure it's a good idea to include it into your releases. Let me know if you think otherwise.

Viir added a commit to Viir/bots that referenced this pull request Feb 20, 2020
Let the bot finish the session when there is nothing left to do: Finish the session three minutes after the last activity was required. This condition also covers the case where the route led to docking in a station.

See the feedback at Arcitectus/Sanderling#10 (comment)
@Viir
Copy link
Collaborator

Viir commented Feb 20, 2020

The feature to stop the bot after docking is now implemented in the example autopilot bot at Viir/bots@8b7ca99

About the undocking feature: I am not sure this would be compatible with routes containing multiple stations: What would happen after the bot docks at the first station?

In any case, bots are now located in the dedicated repository, at https://github.com/Viir/bots

@Viir Viir closed this Feb 20, 2020
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.

None yet

2 participants