tomjn / Shard

A reprogrammable AI framework for the Spring Engine
https://shard.tomjn.com
17 stars 13 forks source link

Separate C++ and Lua into 2 repositories #102

Open tomjn opened 8 years ago

tomjn commented 8 years ago

Currently this repo contains a native C++ AI, and the lua component. This won't work going forward with gadget AIs as it means duplication.

So I'd like to separate out the lua AI portion and have it as a submodule, so that it has 1 canonical source, rather than a native AI, and a gadget AI copy that need constant merging

@abma what needs doing at the Spring engine side of things to ensure this continues to work? There are reports that the EvoRTS submodule is working but not the BA/BAR submodules, I imagine changes will be needed for that too? What do you recommend?

tomjn commented 8 years ago

/cc @eronoobos

abma commented 8 years ago

what needs doing at the Spring engine side of things to ensure this continues to work?

a stable spring release should be done before changes at the infrastructure delay a release.

basicly not much: i still see me asking myself: why are these lua scripts submodules? wouldn't it be better to directly integrate them into the game and then make shard load it from the game?

tomjn commented 8 years ago

A lua gadget AI has bootstrapping code that the native AI doesn't have and vice versa. Coupled with the fact the gadget AI isn't in a 100% state, and that there are advantages and benefits to a native AI, separating out the AI this way simplifies things and avoids a mess On Tue, 21 Jun 2016 at 16:10, abma notifications@github.com wrote:

what needs doing at the Spring engine side of things to ensure this continues to work?

a stable spring release should be done before changes at the infrastructure delay a release.

  • documentation needs to be updated (wiki, ...)
  • travis / buildbot scripts needs be fixed
  • ...?

basicly not much: i still see me asking myself: why are these lua scripts submodules? wouldn't it be better to directly integrate them into the game and then make shard load it from the game?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Tarendai/Shard/issues/102#issuecomment-227471235, or mute the thread https://github.com/notifications/unsubscribe/AADl55zpYP4Pnh4De4gP3DiefmQ67uaPks5qN_79gaJpZM4I6xY8 .

abma commented 8 years ago

How is the gadget AI then loaded/run? is there an (basicly) working example?

eronoobos commented 8 years ago

https://github.com/eronoobos/ShardSpringLua & https://github.com/eronoobos/Balanced-Annihilation-Shard-LuaAI

abma commented 8 years ago

when the ai is updated a commit has to be made to the game?

(this makes the diff VERY low to having it in the same repo)

eronoobos commented 8 years ago

I don't follow, but you made me realize that while https://github.com/eronoobos/ShardSpringLua/tree/master/luarules/gadgets/ai contains just the base Lua files, https://github.com/Tarendai/Shard/tree/master/data/ai contains the game configurations, too. Making those two directories one repository would still reduce my headache of manually copying from one to the other, but if a game repository on github wanted to use that repo as a submodule, they would have to make a pull request just to update their own config, even if that config was its own repository. That mess seems worse than the current mess.

I suppose, @Tarendai, the directory structure could be changed to something like ai/base/ and ai/config/, if we changed shard_include? Not sure if that's a good idea or not

abma commented 8 years ago

I don't follow,

the game depends on a specific ai version: when the ai needs an update, the game has to be updated, right?

abma commented 8 years ago

in other words:

what needs to be done for the

  1. ai dev
  2. game dev
  3. engine dev
  4. player for

for updating the ai?

eronoobos commented 8 years ago

when the ai needs an update, the game has to be updated, right?

yes, of course. but that's true of any Lua AI

if we're talking about the C++ Skirmish AI, then it's the engine that needs to be updated

tomjn commented 8 years ago

For the moment the native C++ AI is the prime version of the AI, and the most fully featured. This may change in future, but we have a AI, written in lua, that works with an API, and 2 implementations of that API ( LuaAI and NativeC++Wrapper ).

Bundling the AI and the Native C++ wrapper is causing organisational and structural issues for the development of the Lua AI implementation ( where most of the future interest in the AI lies as far as the spring engine is concerned ). To maintain momentum I'm eager to proceed as soon as possible.

in other words:

what needs to be done for the

ai dev

Remove the AI from this repo and add a submodule to its new location

game dev

At the moment, nothing, that part hasn't been resolved, and isn't relevant to the engine build process or the Shard native C++ API implementation

engine dev

Adjustment of the build process to ensure that all submodules are pulled in during build process, or reassurance that this is already the case

player for

Nothing, the immediate result for them would be the same as before

for updating the ai?

Business as usual

Content devs who want to use the 100% lua implementation of Shard will have to work with a partial API until it's fully implemented, and will need to take the needed steps to set it up in their games, but this has no bearing on the engines build process. Including the necessary parts to make Shard work as a LuaAI inside this repository would mean additional manual steps, complicated processes, and a significantly higher barrier to entry and support burden. Currently the best version of Shard for gadget LuaAI is in a forked repo that could never be merged directly into this repository, duplicating efforts

abma commented 8 years ago

that doesn't answer my questions: what needs to be done to update (with submodules) in the future when the ai changes for the involved persons?

imo its a lot more work for several people in comparison to having it in one repo.

tomjn commented 8 years ago

Other than git sub module update? So long as the engine always pulls the latest sub modules there shouldn't be an issue. My understanding is that this already happens with EvoRTS On Fri, 1 Jul 2016 at 23:16, abma notifications@github.com wrote:

that doesn't answer my questions: what needs to be done to update (with submodules) in the future when the ai changes for the involved persons?

imo its a lot more work for several people in comparison to having it in one repo.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/Tarendai/Shard/issues/102#issuecomment-230059105, or mute the thread https://github.com/notifications/unsubscribe/AADl53JAKKOYDDROwb_TIn-3HiSiSoDwks5qRZGqgaJpZM4I6xY8 .

abma commented 8 years ago

in my understanding: ai dev needs to update his repository and send a pull request to shard game dev needs to send a pull request to the ai dev engine dev needs to merge pull requests from shard player needs to update engine ?!

so for an update to take place in the engine, ai dev + shard dev + engine dev needs to merge the pull request.

thats a long chain which will lead to large delays when developing imo and is a lot of overhead.

tomjn commented 8 years ago

I'm not sure where these pull requests are coming from, and game developers can release via Rapid and don't require an engine release

The 100% lua AI built with gadgets is irrelevant to this discussion and has no involvement with engine development.

For the native C++ implementation I fail to see how the process is different from an engine perspective. Shard is already synced with the engine releases, I'm not sure how the project is managed internally is relevant.

We have but 1 issue. If Shards C++ implementation uses a submodule, will that be pulled in via the engines build process, and the submodules inside that. Or will we end up with a Shard that complains at startup that no lua files are present? Will it still pull in the EvoRTS and BAR/BA submodules?

On Sat, 2 Jul 2016 at 13:04, abma notifications@github.com wrote:

in my understanding: ai dev needs to update his repository and send a pull request to shard game dev needs to send a pull request to the ai dev engine dev needs to merge pull requests from shard player needs to update engine ?!

so for an update to take place in the engine, ai dev + shard dev + engine dev needs to merge the pull request.

thats a long chain which will lead to large delays when developing imo and is a lot of overhead.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Tarendai/Shard/issues/102#issuecomment-230098557, or mute the thread https://github.com/notifications/unsubscribe/AADl58rL1SsMPsW6_s0oGNhpY0Hafs2cks5qRlO2gaJpZM4I6xY8 .

abma commented 8 years ago

We have but 1 issue. If Shards C++ implementation uses a submodule, will that be pulled in via the engines build process, and the submodules inside that. Or will we end up with a Shard that complains at startup that no lua files are present? Will it still pull in the EvoRTS and BAR/BA submodules?

submodules (as Shard itself is) are optional. so the only thing you could do to enforce the presence of a submodule is to add some check into cmake for the presence of a subdirectory.

tomjn commented 8 years ago

Do we not already have that? EvoRTS specific logic is in a submodule

My knowledge of CMake is near nonexistent, as is the documentation, and I get the feeling the scope I'm asking this question in is a tiny subset of the scope you're thinking of.

Everybody here is in agreement, we have a plan and would like to execute it. But the spring engines CMake system is an unknown, we do not know if implementing our plan will break Shards Native C++ implementation in the build process or what changes in CMake would be needed and how to implement them On Sat, 2 Jul 2016 at 15:03, abma notifications@github.com wrote:

We have but 1 issue. If Shards C++ implementation uses a submodule, will that be pulled in via the engines build process, and the submodules inside that. Or will we end up with a Shard that complains at startup that no lua files are present? Will it still pull in the EvoRTS and BAR/BA submodules?

submodules (as Shard itself is) are optional. so the only thing you could do to enforce the presence of a submodule is to add some check into cmake for the presence of a subdirectory.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Tarendai/Shard/issues/102#issuecomment-230103145, or mute the thread https://github.com/notifications/unsubscribe/AADl51W0ETWvFxpAcU5Ttl6iBi8u5IP5ks5qRm-qgaJpZM4I6xY8 .

abma commented 8 years ago

i don't understand your plan. i only know about "split into submodules" but there are IMHO more reasons against doing that then reasons for doing that.

tomjn commented 8 years ago

This isn't a discussion over wether it's a good idea or not, though if you have a better plan please suggest it in a new issue. I don't know how the engines build process will react, will it pull in the submodules? If it doesn't pull in the submodules then it's broken.

Specifically, the plan is to move everything in data/ai/ into a new repository. This way Shards logic is independent from the API implementation.

Right now if you want to use it in a lua AI in your game, you've got all this native C++ code you don't need. If you wanted to use the native C++ AI, there's all this lua AI gadget stuff you don't need. It's messy and complicated, and makes updating things complicated. If you're say the BAR developer, and you want a pure 100% lua version of Shard, you've got to grab this repo, take the stuff out the data/ai folder, delete all the Visual Studio/CMake/C++ stuff, and hem it into the other repo that has the LuaAI gadget loading code. It's a bit of a mess.

The end result of this is that we'd have 3 repos:

Now AI development can happen in 1 place. Right now there are 2 versions of Shard with duplicated code that cannot be merged together

Ignoring why we might want to do this, or the workflow or processes that might result of it, will this work with the engines CMake build system, or does it require work to be done? Assuming zero knowledge of CMake and the engines build process

abma commented 8 years ago

will it pull in the submodules

no, it requires a lot of changes to the build scripts. also all build instructions are missing the --recursive parameters.

will this work with the engines CMake build system, or does it require work to be done?

yes, as it must check if the submodules are checked out / exists. else it will fail at some later point / the submodules are just missing.

tomjn commented 8 years ago

Where would one begin in order to implement that? Or is there an alternative dependency management system that can be used, similar to composer/bower/npm?

abma commented 8 years ago

Where would one begin in order to implement that?

https://github.com/Tarendai/Shard/blob/master/CMakeLists.txt#L10

example: https://github.com/spring/spring/blob/develop/tools/CMakeLists.txt#L5

i don't see alternatives to cmake which make sense as they would require a new tool.

abma commented 8 years ago

i've reverted this in spring's fork:

https://github.com/spring/Shard

submodules cause a lot more problems than they solve.

tomjn commented 8 years ago

What did you revert? None of the changes discussed here were ever made

tomjn commented 8 years ago

Problems can be frustrating but we all contribute our own time for free here, being rude is not the answer, and I'd much prefer that you open a constructive discussion on how to move forward when these problems occur.

With that said, I'm disappointed as the defacto project lead of the engine you've barged in here with unfounded and unresearched accusations. It's irresponsible and demoralising, and I don't appreciate you shutting down a discussion on solutions in this thread issue, or derailing it with an unrelated problem that already has a separate issue thread ( #109 ).

Until you learn good conduct befitting of a project lead I've revoked your admin status on here, I see no reason you can't make pull requests to feed back fixes made elsewhere. If you need to vent, GitHub is not the place for it.

abma commented 8 years ago

What did you revert? None of the changes discussed here were ever made

because i don't want to break it a second time i "removed" the submodles.

tomjn commented 8 years ago

Ideally Shard would use a project level dependency manager, such as how PHP has Composer, Javascript NPM, etc etc, letting dependencies be pulled in at build time with success and failure conditions, mirrors, alternatives, etc, rather than as less robust git submodules that need updating.

I'm unaware of any that would do the job here though, perhaps you might know of something that would do the job? Something I can specify in a file what packages, their versions, and locations they can be found

abma commented 7 years ago

just use a single git repository and no submodules. KISS! (!!!!)

tomjn commented 7 years ago

That doesn't work, the C++ AI and the LuaRules AI are different projects with a shared dependency. Game devs aren't going to bundle a Visual Studio project in their games and the native AI doesn't need a LuaRule handler, they also have different requirements about the root of the repository, the current single repo situation is far from KISS

abma commented 7 years ago

submodules are a mess:

the only clean way to solve your and my issues is to make shard to be able to load the lua scripts from the current loaded game archive.

also for the current situation: remove the submodules and make shard work again or i/we have to remove shard from spring as its not useful when its broken.

tomjn commented 7 years ago

Forboding added the relevant repo back to his GitHub repo, and you yourself removed the submodule from Shard, so it shouldn't be broken. If it is currently broken, nobody has made me aware of it, or filed any bug reports.

As for submodules, as I said, please provide a better solution than submodules, you can pooh pooh it all you like but if it's the only solution available what choice is there? Besides, we don't need to implement it, you'll assume we did all the work and complain anyway.

At the moment there are only 2 submodules:

The lua in those submodules isn't going to break unless I make massive changes to how Shard works, and @eronoobos is still active, so they are maintained.

the only clean way to solve your and my issues is to make shard to be able to load the lua scripts from the current loaded game archive.

There's no API for this, I've been saying this for years and I refuse to do voodoo trickery to a totally undocumented API that even the author doesn't fully understand to get it. What I have right now works

I understand you dislike submodules, and have done since you first suggested it. What bothers me though is you're not even attempting to problem solve, you're just railing against submodules. Even the most stupid of people would realise you can just download a tagged release archive from GitHub and unzip it into a subfolder, hey presto submodule problem solved.

The only problem is I know almost nothing about CMake and springs build system, and haven't the time to learn. You on the other hand could have done it 10x over, or even indicated where to look and what to do, but decided to argue and hurl burning rocks at bridges instead.

also for the current situation: remove the submodules and make shard work again or i/we have to remove shard from spring as its not useful when its broken.

Again, you yourself removed the submodule

abma commented 7 years ago

yeah, because they broke the build of spring.

now i removed shard because it just didn't work.

tomjn commented 7 years ago

If Shard breaks Springs build process, the correct course of action is to ping me privately then open an issue. I personally receive thousands of GitHub emails each day of which my paid work takes precedence, I don't always see Shard related issues immediately. A DM via Twitter/Facebook/Slack/Skype is the best way to get my attention ( in that order )

I believe the issue you're referring to is #111, which is completely unrelated to submodules. I've committed a fix in https://github.com/tomjn/Shard/commit/228026ce2ddc4558104ebf092b54e26c0865cd1f

tomjn commented 7 years ago

As an aside, if you have validation tests that run on Shard, I should be included and notified when they fail

abma commented 7 years ago

you should listen to someone who knows how the springs build system / infrastructure works...

As an aside, if you have validation tests that run on Shard, I should be included and notified when they fail

thats currently not possible.

I believe the issue you're referring to is #111, which is completely unrelated to submodules. I've committed a fix in 228026c

before #111 it broke because of submodule removals / renames. now even the main shard repository was renamed. sorry i don't see why i should discuss any further as long i get ignored.

so in summary shard broke at least 4 times the build process or the validation tests. also building an old git version (i.e. current spring master version=spring 103.0) is broken of this.

i've lost patience: you don't understand how urgent it is to NOT break the build process / validation tests.

tomjn commented 7 years ago

This is the first I've been made aware of those breakages, I can see how changing my GH username would break things in hindsight but I was not told the breakage had occurred, it's been months and I could have had that fixed, why was I not informed until now?

Even then that only gives me 3 build breaks, of which I am not responsible for the first and the latter 2 I have only just become aware of, the 4th I am still unaware of. I can't fix things if nobody tells me about them!

As for validation tests, pipe the failed results to an email mailing list people can subscribe to. At that point I can use GMail filtering magic to bubble any that mention Shard as the culprit to the top

Better yet, why did nobody request tagging? Would it not have made more sense for Spring to track a stable version rather than the bleeding edge dev versions of Shard?

tomjn commented 7 years ago

github.com/tarendai/shard is back now

tomjn commented 7 years ago

@abma I have an idea. If I split this repo into 2 repos:

Then have both included in Spring, and have the runtime look one folder up and down into an ai folder, would this be acceptable? This would eliminate the 2 projects in 1 repo problem, let those who want to work on putting Shard inside their game do so without dealing with the C++ part, and not have nested git submodules inside Shard itself

What are your thoughts?

abma commented 7 years ago

it would be still very complicated to maintain Shard.

a dev who wants to add some new function to shard api needs to change 3 repos: spring + lua + shard itself.

let those who want to work on putting Shard inside their game do so without dealing with the C++ part,

shard must be able to load lua files from the game archive, thats the only solution IMHO.

abma commented 7 years ago

also having lua files bound to engine version (which your suggestion still is) still makes it impossible to update Shard's data/lua files without updating the engine.

tomjn commented 7 years ago

Not really, this repo would only contain the C++ part, which is stable and has seen relatively little change in the last few years.

The second repo would contain the Lua AI, that sees the most change

We can then create a third repo that bootstraps and implements gadget APIs with a submodule or whatever, the end result for a game dev is always the same, a LuaAI.

In terms of investment, our differing views present these options:

Keeping the legacy C++ AI wrapper and the generic AI code written in lua in the same repo is holding the entire project back and poses major logistical problems. I'm not at a point where the C++ portion of Shard can be jettisoned completely, but I need a solution to that logistical problem. What you're suggesting would lock Shard into being a native AI forever, nevermind if a Starcraft or other engine implementation was built ( which was the whole point of Shard in the first place ).

tomjn commented 7 years ago

To make it clear, this repo contains 2 projects:

The first can be used with the second, or it can be used with a 3rd project, the LuaAI bootstrapper that goes inside an archive. The AI and the bootstrapper need separating so that the bootstrapper can be replaced and deprecated.

If I were to make it load from the VFS, I'd need to get all the content devs to update their games, many of which are no longer maintained. In the past they've not been welcoming to these things, and if I do this now they're all going to turn around and say native AIs are dead, use a 100% LuaAI instead. So loading AI content from the VFS is a nonstarter, and expensive to implement

abma commented 7 years ago

If I were to make it load from the VFS, I'd need to get all the content devs to update their games, many of which are no longer maintained.

no need to get all content devs to update their games: if data exists in the game archive, use this one, if not, try the shard's data dir.

The AI and the bootstrapper need separating so that the bootstrapper can be replaced and deprecated.

i don't understand. I guess you want to have the option to build shard for an other game/engine? if so, you could i.e. use a cmake option to switch between the build types. i still don't see why the split is a must.

still the plan of making shard beeing used in different games/engines is weird in the context of https://springrts.com/phpbb/viewtopic.php?f=1&t=32438

tomjn commented 7 years ago

I can see how that would make sense from an overrides perspective, but that's a different problem. Also a lot has changed and happened since 2014 :p

Right now for Shard to be a LuaAI requires changes that are incompatible with the Shard in this repo, the 2 aren't 100% compatible. I've made changes to try and bring them together, but including it in BAR for example would still result in the source for the C++ API being included inside the BAR archives, which is silly

I haven't the expertise to build cmake out to wrangle all of that, and I doubt the content devs do either. They'll want to make changes and create forks too. An obvious answer is to make the data/ai folder a submodule but you've made your opinions on that clear

tomjn commented 7 years ago

@abma is it possible for cmake to fetch the contents of another branch into a subfolder?

abma commented 7 years ago

possible yes but this makes the process of building spring from a git repository / from scratch VERY fragile. also tarballs must be created, too: there is a lot of stuff which must be tested when doing this change.

i.e. at which point should the file beeing downloaded? should it be downloaded every time when spring is build? (if not, this could make rebuilding spring very slow), ...