SWY1985 / CivOne

An open source implementation of Sid Meier's Civilization.
http://www.civone.org/
Creative Commons Zero v1.0 Universal
245 stars 49 forks source link

Add files via upload #490

Open EnockNitti opened 6 years ago

EnockNitti commented 6 years ago

Adding AStar for improved function when using "goto" command

SWY1985 commented 6 years ago

Hi! Thanks for your Pull Request. I would love to merge the AStar code into CivOne, but it would need to be an optional patch that can be enabled via the Setup screen, with the 'original' algorithm being the default. If you need pointers on how to make these changes, please let me know.

EnockNitti commented 6 years ago

Ok A pointer would be nice Thanks

ons 22 aug. 2018 kl. 15:44 skrev SWY1985 notifications@github.com:

Hi! Thanks for your Pull Request. I would love to merge the AStar code into CivOne, but it would need to be an optional patch that can be enabled via the Setup screen, with the 'original' algorithm being the default. If you need pointers on how to make these changes, please let me know.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-415036616, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvUM7EHxpmfMJjDjM1eYczg8pGufqks5uTWA9gaJpZM4V9vIM .

SWY1985 commented 6 years ago

In the file src/Settings.cs, you can add a patch setting, according to existing patches: https://github.com/SWY1985/CivOne/blob/b6db419de3baa6f5a91a3ffa526218d55c79a9d8/src/Settings.cs#L133-L229

In the file src/Screens/Setup.cs, you need to add a menu entry for the patch: https://github.com/SWY1985/CivOne/blob/b6db419de3baa6f5a91a3ffa526218d55c79a9d8/src/Screens/Setup.cs#L176-L235

It would be best to create a new Enum for use with your patch, for example:

public enum PathFindingAlgorithm
{
    Default = 0,
    AStar = 1,
}

Then, you can create a switch statement where you check the value of Settings.{yourSettingName} or Settings.Instance.{yourSettingName}, and add the current en the new code under the appropriate case.

I think this should work.

EnockNitti commented 6 years ago

Ok I will take care of this when I am back home next week

Regards

tors 23 aug. 2018 kl. 13:51 skrev SWY1985 notifications@github.com:

In the file src/Settings.cs, you can add a patch setting, according to existing patches: https://github.com/SWY1985/CivOne/blob/b6db419de3baa6f5a91a3ffa526218d55c79a9d8/src/Settings.cs#L133-L229

In the file src/Screens/Setup.cs, you need to add a menu entry for the patch: https://github.com/SWY1985/CivOne/blob/b6db419de3baa6f5a91a3ffa526218d55c79a9d8/src/Screens/Setup.cs#L176-L235

It would be best to create a new Enum for use with your patch, for example:

public enum PathFindingAlgorithm { Default = 0, AStar = 1, }

Then, you can create a switch statement where you check the value of Settings.{yourSettingName} or Settings.Instance.{yourSettingName}, and add the current en the new code under the appropriate case.

I think this should work.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-415387493, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvWBDHQ7PIYJlxl1RvAbh4zcuECs4ks5uTpczgaJpZM4V9vIM .

EnockNitti commented 6 years ago

Hi I have added an entry for AStar smart "goto" in the debug menu. Have tested and I have not found any performance issues. But I have found a few othe issues: A unit (and the game) might be stuck when using "goto" ( with or without AStar ) It happens if the unit har moves left. Check screen dump. The only way to cancel a "goto" is to set a "goto" to the units position It is possible to "goto" a position not visible on the map. When I hit "g" for goto there is 50% chance that the unit becomes invisible ( the flashing stops in the "dark" phase )

[image: gotostuck.jpg]

On Wed, Aug 22, 2018 at 3:44 PM SWY1985 notifications@github.com wrote:

Hi! Thanks for your Pull Request. I would love to merge the AStar code into CivOne, but it would need to be an optional patch that can be enabled via the Setup screen, with the 'original' algorithm being the default. If you need pointers on how to make these changes, please let me know.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-415036616, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvUM7EHxpmfMJjDjM1eYczg8pGufqks5uTWA9gaJpZM4V9vIM .

EnockNitti commented 6 years ago

A question: It seems like the "Reveal world" patch stays on permanent if set to "yes". I was unable to find out how this was accomplished. How is it done ? I would like to add that feature to "smart PathFinding"

On Wed, Aug 22, 2018 at 3:44 PM SWY1985 notifications@github.com wrote:

Hi! Thanks for your Pull Request. I would love to merge the AStar code into CivOne, but it would need to be an optional patch that can be enabled via the Setup screen, with the 'original' algorithm being the default. If you need pointers on how to make these changes, please let me know.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-415036616, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvUM7EHxpmfMJjDjM1eYczg8pGufqks5uTWA9gaJpZM4V9vIM .

SWY1985 commented 6 years ago

Hi! I'm sorry I didn't have time to review your changes yet. Hopefully tomorrow.

I had a quick look. The PathFinding setting gets saved, but not loaded on startup. You should add the GetSettings code in the Settings constructor, here:

https://github.com/SWY1985/CivOne/blob/b6db419de3baa6f5a91a3ffa526218d55c79a9d8/src/Settings.cs#L392-L424

EnockNitti commented 6 years ago

Thanks for your pointer, I must have been tired when I was trying to find out how the loading was done. Change is committed Some problems when comparing setup.cs and settings.cs. Something with line endings or what ??

On Thu, Aug 30, 2018 at 7:31 AM SWY1985 notifications@github.com wrote:

Hi! I'm sorry I didn't have time to review your changes yet. Hopefully tomorrow.

I had a quick look. The PathFinding setting gets saved, but not loaded on startup. You should add the GetSettings code in the Settings constructor, here:

https://github.com/SWY1985/CivOne/blob/b6db419de3baa6f5a91a3ffa526218d55c79a9d8/src/Settings.cs#L392-L424

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-417194834, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvSBC8g-J9kP88mF6-l69IIYUJM7jks5uV3ibgaJpZM4V9vIM .

SWY1985 commented 6 years ago

I noticed this too, it looks like GitHub treats them as new files, and on GitHub it looks like the files in the root folder instead of the "src" folder, but that's not what I see when I look at your repository. I don't know why it's doing that.

If you've used VScode, the line endings should be correct.

Can you please add (copy) the file header to the new .cs files that you've added? I think I'll be able to review and merge your code tomorrow evening.

EnockNitti commented 6 years ago

Not much action with CivOne lately... Just as well. I have done some more work on the smart "goto".

The code is rearranged, Maps.cs is back as before, All code is moved into one file: AStar.sc. Have added AStar for UnitClass.Water too. Trireme was a bit tricky, to make sure the Trireme always ends close to land at end of turn and still able to cross all 3(4) steps gaps without getting lost. Also made Trireme to leave land whenever it could be done safely. Also added: Avoiding enemy units when pathfinding. Due to the bug that "goto" is not canceled when meeting an enemy, I implemented that function in my code, so at least units exits "goto" when using smart "goto". Added file header Don't think performance is a problem. Only a small delay when pathfinding really difficult cases. Even on my old system Actually I am quite impressed by the capability of the AStar algorithm.

When loading some old saved games to test my code I hit on a problem. In some cases I got an exception caused by a not existing unit number( 74 i think is was ) when reading fortified units from the saved game. A minor fix in GameLoadSave.cs. Maybe that unit number signifies something, I don't know. If you want a saved game for test, just let me know.

Another bug: "h" (home) do not work for bombers.

Line endings: I made a check on your files. You are using line-endings 0aH "LF" in your files( Unix style ), My files are using 0dH 0aH "CR/LF" ( Windows style ) Visual studio seems to not care. I have now saved my files with Unix line-endings.

I did mess up my resp, so I deleted it and created a new. Might be a cause of some confusion.

Have a another problem that you might have a solution to: I made the "mistake" to update VS to 15.8.3 and now I don't know how to publish my version of CivOne. The entry for "publish" is gone in the menu system.

Btw, this has been my first experience with C#. It is a good way to learn a new language to have a Small project like this, and have some code to read and learn from.

Regards

On Thu, Aug 30, 2018 at 11:51 AM SWY1985 notifications@github.com wrote:

I noticed this too, it looks like GitHub treats them as new files, and on GitHub it looks like the files in the root folder instead of the "src" folder, but that's not what I see when I look at your repository. I don't know why it's doing that.

If you've used VScode, the line endings should be correct.

Can you please add (copy) the file header to the new .cs files that you've added? I think I'll be able to review and merge your code tomorrow evening.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-417261111, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvU6ue-J56c65iff5X3P3Eg1FFxY3ks5uV7WXgaJpZM4V9vIM .

EnockNitti commented 6 years ago

Hi

I just want to say that I have reworked the function "private void CalculateContinentSize()" using an another algorithm. This since your comment indicated that it was some problem with it. As far as I can see my algorithm gives a correct size of all continents and numbers them in size order.

Made some more work with "smart" goto. Air units has got some special treatment. ( Found a bug too 😮 )

Are you still working with this project or has other aspects of life got higher priority ?

Regrds John Rehn aka enock nitti

SWY1985 commented 6 years ago

Are you still working with this project or has other aspects of life got higher priority ?

Hi John,

At the moment, work on this project is not the highest priority for me. The project is not dead, but I've got loads of other things (mainly my work) taking up my time. I'm so sorry. I'll try to review your code later this week, and I am hoping (intending) to get some work done this month.

EnockNitti commented 6 years ago

Ok No problem.

Well... One problem: When I do "compare" I get a result that indicates that Map.sc and units/Baseunit.cs are new files. I don't understand why. Maybe you can grab them "by hand" if necessary.

An another thing that I have seen during my test of goto, it is sometimes hard to see what happens with the units during movement. Sometimes it looks like they are "teleported" to the target. This is handled in a part of the project that I don't understand very much about, so it is hard for me to do something about it.

I have also seen the problem where Improvements placed on ocean, and in one case settlers was placed in the ocean too. Will try to reproduce the problem.

regards

On Mon, Oct 1, 2018 at 8:49 AM SWY1985 notifications@github.com wrote:

Are you still working with this project or has other aspects of life got higher priority ?

Hi John,

At the moment, work on this project is not the highest priority for me. The project is not dead, but I've got loads of other things (mainly my work) taking up my time. I'm so sorry. I'll try to review your code later this week, and I am hoping (intending) to get some work done this month.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-425805648, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvfK8zj4Npxnj-PZ3xsR0CS2NHyjNks5ugbrggaJpZM4V9vIM .

EnockNitti commented 6 years ago

Hi

Have got a example of the "Strange maps" problem.

Here is 4 cases of the same world in different stages of "strangeness" CIV3 is my original I used for some testings, no problems as I can see. CIV1 not too bad but there is a hole in the Antarctic at the "Dateline" CIV2 Some strange improvements at sea. CIV0 Really bad. Continents are shifted to the west.

In the first 2 cases I did not detected the error until I saved and reloaded. In the last case the shifting happened while I was testing. My first hypothesis was that the error happens during saving, but the last case contradicts that.

All 4 maps are in the attached zip.

If there anything I can do to help, let me know.

regards John Rehn

SWY1985 commented 6 years ago

There are known problems with loading/saving games. Can you report a bug for this, please?

EnockNitti commented 6 years ago

CIV0 Really bad. Continents are shifted to the EAST.

SWY1985 commented 6 years ago

Please, report a bug for this: https://github.com/SWY1985/CivOne/issues

EnockNitti commented 6 years ago

I going to.

But I will do some more analysis of the problem first. As I have some good examples of the problem. Right now "prime suspect" is LZW. I have worked before with that algorithm I know how easy it is to mess up.

On Wed, Oct 3, 2018 at 9:43 AM SWY1985 notifications@github.com wrote:

Please, report a bug for this: https://github.com/SWY1985/CivOne/issues

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-426541892, or mute the thread https://github.com/notifications/unsubscribe-auth/AlfkvXnmnyGBR1j4DajAxNlhXMG2mE6gks5uhGqvgaJpZM4V9vIM .

SWY1985 commented 5 years ago

Hi, I'm so sorry, I haven't had a lot of time lately. I'm getting back into the code and I will merge your code soon.

EnockNitti commented 5 years ago

Hi Nice to see you back If you have any questions about my code I'll be glad to help you.

On Wed, Apr 10, 2019 at 6:40 AM SWY1985 notifications@github.com wrote:

Hi, I'm so sorry, I haven't had a lot of time lately. I'm getting back into the code and I will merge your code soon.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SWY1985/CivOne/pull/490#issuecomment-481530767, or mute the thread https://github.com/notifications/unsubscribe-auth/Alfkve1x7vf6B7pAUelGahTpfYCwuRkwks5vfWtFgaJpZM4V9vIM .