Author Topic: Small Git-Question about pull-requests  (Read 6091 times)

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Small Git-Question about pull-requests
« on: October 28, 2022, 12:24:37 pm »
When I commit something to a branch on which there is an unaccepted pull-request, will the pull-request be updated automatically or do I have to retract the pull-request and do a new one?

Offline Yankes

  • Commander
  • *****
  • Posts: 3207
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #1 on: October 28, 2022, 12:38:43 pm »
All PR "target" most recent version. If there are conflict with current version there will be message informing you about this.

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #2 on: October 28, 2022, 01:50:16 pm »
Okay, thanks.

Follow-up-question:

What is the intended way to inform the receiver of the PR when I think I'm actually done?
Will they get an update when I commit new code? Do I just use completely different ways to tell them?

Offline Yankes

  • Commander
  • *****
  • Posts: 3207
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #3 on: October 28, 2022, 02:06:03 pm »
It should be creation of PR represent that you think this is finished :>

And after you created PR there are info about if you change code in given PR.

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #4 on: October 28, 2022, 03:17:57 pm »
It should be creation of PR represent that you think this is finished :>
Yeah, I agree. But on the other hand what I'm trying to accomplish is more of an iterative ongoing process. For my last project I worked almost 2 years on the AI. Making it a little better from version to version.
As long as the main-author also was still working on the project he basically told me when to do pull-requests and I just included as much as I had gotten done until then.

In this case my pull-request already included something that felt like a workable first iteration. But I realized, shortly after doing it, that it basically cheats in a blatantly obvious way. And I don't really want people to think that giving the AI the ability to shoot at soldiers it can't even see is my understanding of improving AI. :o

That could be a separate option though with a respective warning.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 8616
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #5 on: October 28, 2022, 04:07:58 pm »
Before you spend too much time on it, here's the OXCE disclaimer:

1. only fully finished features -- if you need to work iteratively, that's what your fork is for... you can do it whatever way you like, publish test builds and at the end squash everything into one final commit
2. only backwards-compatible features -- there must be either a switch or modding ruleset to turn everything on/off
3. only features, which belong to OXCE scope: https://openxcom.org/forum/index.php/topic,5251.0.html , https://openxcom.org/forum/index.php/topic,6498.0.html -- your "complete rework" of AI already borders significantly on being outside of the OXCE scope

4. and most importantly, only features I like (or Yankes likes) or at least tolerate -- WE will have to fix, maintain, support and document all of this... and that's not for free... and also it's a very ungrateful job (we regularly have people complaining about everything, sometimes in a quite hurtful manner)

From what I read so far, I can't say I'm happy about your proposal.
It sounds like "my take on ideal xcom AI"... which is not useful/practical for me, since every person on the planet has a different opinion on that.
Even if you manage to make it exactly to your expectations, practically everybody else will either say it could be much better (or that it is too good).

Same as Yankes wrote here (https://openxcom.org/forum/index.php/topic,10834.msg150011.html#msg150011), what I could tolerate are small optional improvements that can be enabled/disabled and somewhat tweaked by modders to their liking.

If you want a big bang AI rework, there's a very high chance the disadvantages will simply outweigh the advantages for me.
You might have bigger success with such PRs in other OXC forks... for example Finnik's FTA.
« Last Edit: October 28, 2022, 04:12:07 pm by Meridian »

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #6 on: October 29, 2022, 03:55:56 am »
1. I just wrapped up something, that I think qualifies as a state that I'd like some player-feedback for.

I would indeed like to publish a test-build. However, I'm not sure how exactly to do that. I can easily run it from the editor, with some trouble from the bin\Win32\Release-folder but when I put the exe in the same folder as the original OpenXcom-installation and try to run it, nothing happens.

So what I'd like is to be able to build an exe, that could simply be put into the OpenXcom-folder. A complete installer would be even better. For Remnants of the Precursors some really able Git-User made it so, that I have a workflow-action under https://github.com/Xilmi/Rotp-Fusion/actions where I just need to click "Xilmi Release" and "Run Workflow" and it build the entire distributable package all by itself and puts a download-link to https://github.com/Xilmi/Rotp-Fusion/releases

Something like that would be absolutely lovely but I guess also requires a lot of knowledge of how build-automation works and a lot of dedication to set it up.


2. I reworked everything so it can easily be switched on and off. There's now basically a single hook in the "think"-function after variable-initalization that calls my code instead:

Code: [Select]
if (Options::brutalAI && _unit->getFaction() == FACTION_HOSTILE)
{
brutalThink(action);
return;
}

This is all that remains there. The rest of the new code is all added to the end of the file.
The game should behave exactly like it always has when the feature is switched off. So I'd say that backwards-compatibility is a given.


3. When I read the notions about the scope correctly, then due to the new AI being entirely optional, I don't see a violation of said scope. Nothing that worked previously got removed or changed. It's only added.


4. Well, I obviously cannot make someone like AI changes, if they are not open for such changes in the first place. I also would say that I feel kinda responsible for any complaints and requirements for fixes or other forms of maintenance within a feature that I added myself. Same goes for documenting. If you'd point me to an example of how other stuff is documented, I'd be willing to provide a similar style of documentation for what I've done so you just need to paste it into your documents.


I kinda have some reputation as being "the AI guy". I've done AI work for "Pandora: First Contact", "Dominius Galaxia", a "StarCraft"-Bot, a little bit for "Warhammer 40k: Gladius" and most recently primarily for "Remnants of the Precursors". I mean it's pretty clear that when I code an AI, that it's "my take on" what I think the AI should be like. For games with diplomacy or more than 2 factions it's definitely more tricky. For X-Com my thought process was, that maybe some players might want a greater challenge but without just upping the stats and/or numbers of the aliens. So it's pretty straight-forward, that I'm attempting to make the AI be good at killing X-Com-soldiers. I'm not an expert in X-Com-Games so I'm also very open for suggestions on how to achieve that goal better. That's also why I'm eager for player-feedback. So they can tell me when the AI did something contradictory to that goal and how it should have acted instead.

I'm fully aware that such goal cannot be achieved within a couple of days. For rotp I took in player-feedback over the course of almost two years. I attached a screenshot of a great compliment I got for my efforts. I basically ran out of feedback about things that I could still improve and that's why I'm looking for other pastures. I stumbled over OpenXCom due to a video called "RetroAhoy: X-Com" ( https://www.youtube.com/watch?v=gBu77h2FSCM ) and thought this could be a neat new project for me.

Because of the way I worked on previous games, the scope of AI-development and the requirement for feedback, I do indeed see a little issue with the "only fully finished features"-policy. It is much harder to get feedback for it, when it's difficult to reach the player-base. So to me the most promising option seems to be "featured" in the most popular version and I'm willing to jump some hoops to make it work. So I'm open for any suggestions and requests that increase the chances for that to happen.

As previously admitted the first pull-request was indeed premature and very "ugly" in the sense that hooks to my code were spread all over the place instead of neatly tucking it away separate new functions. I think I've taken the justified criticism for that to heart and my modifications since are now in a much better state.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 8616
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #7 on: October 29, 2022, 01:45:08 pm »
Your argumentation is spotless; I understand what you're saying and there's nothing to argue about there.
It's just opinion vs opinion; preference vs preference.

I will not accept a WIP feature into OXCE main branch, but I can offer help in some ways.

We have similar examples from the past (and even the present):
1. example from the past: Soldier Diaries and Commendations (by Shoes): https://openxcom.org/forum/index.php/topic,1718.0.html -- it took more than two years before this feature was merged into OXC; in the meantime Shoes worked on his fork, provided test builds, the community tested, gave feedback, even created mods for it -- but the merge happened only after the feature was well known and well accepted by the community
2. example from the present: In-game Map Editor (by ohartenstein23): https://openxcom.org/forum/index.php/topic,8711.0.html -- ohartenstein23 donated tons of features, which were already merged into OXCE, but even he is not immune against the "no WIP features" rule; he's been working on the map editor for well over 6 months if I'm not mistaken, and the feature also got significant popularity... the work has stopped however, the feature is still WIP and I don't know what will happen to it in the future... but I know that it is a good thing that it didn't make it into OXCE in such unfinished state

Hopefully this illustrates that I'm not treating you worse than anybody else before you.
(PS: there's of course more than just 2 examples, in case you're wondering: the zip loader, android port, sdl2 upgrade, mod.io integration, etc. etc.)

As for the help I can offer:
a/ if you want bigger visibility of your feature on the forum, I can make your main thread "sticky"
b/ if you want bigger visibility of your code on github, I can create a separate feature branch for you, where you can make PRs with WIP code too
c/ if you want help with builds, I can create builds for you from the feature branch in b/ -- alternatively, you can ask Stoddard (the maintainer of our build farm) to create a build farm also for your fork
d/ if you want bigger visibility of your test builds, I can offer you a spot in my main download thread, right next to the official OXCE downloads

That's already more than what I offered to anyone else so far... however, the community support, community testing, feedback, approval and blessing is something only you alone can do.

And if your feature gets popular enough (and finished), and I'm still around by then, I will consider merging it.
« Last Edit: October 29, 2022, 01:50:39 pm by Meridian »

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #8 on: October 30, 2022, 10:32:36 am »
Sounds awesome! I'd be totally fine with that approach!

I think I can take care of the "advertisement"-part myself but of course wouldn't mind if you'd use your reach for it either. I just need a better distribution-method than "fork from my github and compile it yourself".

So the idea about the feature-branch and creating builds for it sounds about perfect for my purposes. Just something where I can say: "Wanna try X-Com with a new experimental AI for the aliens? Here's the link to the installer."

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 8616
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #9 on: October 30, 2022, 11:58:35 am »

Offline Finnik

  • Colonel
  • ****
  • Posts: 492
  • Finnik#0257
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #10 on: October 30, 2022, 02:05:13 pm »
For build's I can also recommend using GitHub tools, @pedroterzero managed to set up the workflow for mine repo: https://github.com/723Studio/OpenXcom_FTA/actions/workflows/ci.yml
Although, @Stoddard configured building FtA using his Jenkins, I prefer GitHub, because:
1) it provides build logs, sometimes I faced VS compiles it well. Jenkins's logs are stored on Stoddard's server, and I don't have access to it.
2) it also builds macOS version

As @Meridian mentioned, I have active fork of OXCE. But it is not a general modding platform, like OXCE, it rather is focused on a single mod - From the Ashes. However, it is mostly backwards compatible to OXCE and supports all its mods. But this requirement is not that strong, like Meridian's, if I will have good arguments, I can make some stuff not backwards compatible.

And I am excited on expanding AI, in case your work (or part of it) would be rejected to be merged in main OXCE branch, it might have a second chance =)

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #11 on: October 31, 2022, 09:29:12 pm »
Here's your feature branch: https://github.com/MeridianOXC/OpenXcom/tree/new-ai
Just did a pull-request towards this branch. :)

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #12 on: October 31, 2022, 10:41:44 pm »
For build's I can also recommend using GitHub tools, @pedroterzero managed to set up the workflow for mine repo: https://github.com/723Studio/OpenXcom_FTA/actions/workflows/ci.yml
Although, @Stoddard configured building FtA using his Jenkins, I prefer GitHub, because:
I tried to copy&paste this file into my GitHub-Account.

It seemingly has built some stuff for ubuntu (gcc & clang) but then failed with something else. I guess I need a little more hand-holding to get it to work. :o

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 8616
    • View Profile

Offline Xilmi

  • Moderator
  • Commander
  • ***
  • Posts: 605
    • View Profile
Re: Small Git-Question about pull-requests
« Reply #14 on: November 01, 2022, 02:01:00 pm »
Here's your build: https://lxnt.wtf/oxem/builds//ExtendedTests/BrutalAI-7.8.1.1-test-9dd518701-2022-11-01-win64.7z
Thanks a lot!  :D
I'll spread the word on some places where I think people might be interested.
I'm actually surprised to get it after the conversation I had with Yankes in the review-comments for AIModule.h
I thought that it would be turned down unless I refactored everything to match his expectations... Which were quite different to what I had in mind.