Skip to content

Conversation

@EnockNitti
Copy link

Adding AStar for improved function when using "goto" command

@Solen1985
Copy link
Owner

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
Copy link
Author

EnockNitti commented Aug 23, 2018 via email

@Solen1985
Copy link
Owner

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
Copy link
Author

EnockNitti commented Aug 23, 2018 via email

@EnockNitti
Copy link
Author

EnockNitti commented Aug 29, 2018 via email

@EnockNitti
Copy link
Author

EnockNitti commented Aug 29, 2018 via email

@Solen1985
Copy link
Owner

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
Copy link
Author

EnockNitti commented Aug 30, 2018 via email

@Solen1985
Copy link
Owner

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
Copy link
Author

EnockNitti commented Sep 8, 2018 via email

@EnockNitti
Copy link
Author

EnockNitti commented Sep 30, 2018 via email

@Solen1985
Copy link
Owner

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
Copy link
Author

EnockNitti commented Oct 1, 2018 via email

@EnockNitti
Copy link
Author

EnockNitti commented Oct 3, 2018 via email

@Solen1985
Copy link
Owner

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

@EnockNitti
Copy link
Author

EnockNitti commented Oct 3, 2018 via email

@Solen1985
Copy link
Owner

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

@EnockNitti
Copy link
Author

EnockNitti commented Oct 3, 2018 via email

@Solen1985
Copy link
Owner

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
Copy link
Author

EnockNitti commented Apr 11, 2019 via email

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.

2 participants