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

Nuclues in Editor, Starting cell 1 hex of cytho #693

Closed
wants to merge 19 commits into from
Closed

Nuclues in Editor, Starting cell 1 hex of cytho #693

wants to merge 19 commits into from

Conversation

Dak2896
Copy link
Contributor

@Dak2896 Dak2896 commented Dec 27, 2018

  • Nucleus is organelle that can be addes in editor
  • Starting cell is empy one with 1 hex of cytho only
  • Is possible only to create one nucleus
  • Organelle that need nucleus are locked until you get one

Know issues:

  • From GUI locked organelle are enable, you just cannot add them, make a feedback why you can't or
    disable them
  • There is no icon in editor for Nucleus
  • 1 hex of cyto it's bigger than other IA bacteria (don't know why)

- Nucleus is organelle that can be addes in editor
- Starting cell is empy one with 1 hex of cytho only
- Nucleus is 9 hex instead of 10
- Is possible only to create one nucleus

Know issues:

- If you are creating nucleus and you make undo after you have select it, you can't add it forever, this happens because it's now made by flag variable system when you select nucleus, should be replace by cycling acutal organelle and find if nucleus is present.

- There is no icon in editor for Nucleus
- 1 hex of cyto it's bigger than other IA bacteria (don't know why)
Copy link
Member

@hhyyrylainen hhyyrylainen left a comment

Choose a reason for hiding this comment

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

The file permissions changed on the scripts and they no longer work. So the linux compile is broken and also the formatting checker (and I'm seeing at least a few formatting problems).

@Dak2896
Copy link
Contributor Author

Dak2896 commented Dec 27, 2018

File permissions? for formatting shouldn't be runFormattingCode automatically change indentation ecc?

@hhyyrylainen
Copy link
Member

kuvakaappaus - 2018-12-27 16-32-48
The scripts don't work anymore because of that.

@Dak2896
Copy link
Contributor Author

Dak2896 commented Dec 27, 2018

Mhh and what is that? doens't it change nothing?

@hhyyrylainen
Copy link
Member

File permissions define on Linux what files can be ran and with wrong permissions everything is ruined.

@Dak2896
Copy link
Contributor Author

Dak2896 commented Dec 27, 2018

Ok, so i have to make fork again and understand why in first commit i had that makeRelease changment

@hhyyrylainen
Copy link
Member

If you are good with Git you could cherry pick the other commits and force push over this PR.

@Dak2896
Copy link
Contributor Author

Dak2896 commented Dec 27, 2018

Is not possible to revert first test commit that make changes on that files?

@hhyyrylainen
Copy link
Member

I guess reverting it might work

@@ -1,4 +1,4 @@
<!DOCTYPE html>
<!DOCTYPE html>
Copy link
Member

Choose a reason for hiding this comment

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

What is the change on this line? Could you check that there isn't the wrong type of line change here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i didn't change nothing at that line, wrong type of line means that changes are in another line?

Copy link
Member

Choose a reason for hiding this comment

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

The only reason I can think of that this line is marked as changed (as there is no visible change) is that the line end character has changed.

EditorAction@ action = EditorAction(organelle.organelle.mpCost,
if((organelle.organelle.name == "nucleus" && !nucleusIsPresent) || organelle.organelle.name != "nucleus")
{
if(organelle.organelle.name == "nucleus")
Copy link
Member

Choose a reason for hiding this comment

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

There are tabs here. Run the formatting script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actyally in my code there are 4 space.. i don't know why when i make push and then pull they change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FAILED to format file: ./ThirdParty/Leviathan/build/ThirdParty/include/bullet/VHACD/test/inc/oclHelper.h

Now when i lunch formatting code i get this, feels bad

@@ -277,8 +277,7 @@ void setupOrganelles(){
Int2(-1, 1),
Int2(-1, 0),
Int2(1, 1),
Int2(0, 2),
Int2(-1, 2)
Int2(0, 2)
Copy link
Member

Choose a reason for hiding this comment

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

Why was the size of the nucleus changed? It's been the same size in hexes forever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like now starting cell was 10 hexes with nucleus, if you add organelle nucleus of 10 hexes size it's 11 hexes, i just remove one to make the situation as was before

Copy link
Member

Choose a reason for hiding this comment

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

But you changed the configuration of the nucleus? That affects everything starting from the generated collision of the cell and how the membrane wraps around things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no i just modify the table, if that is the only one configuration for nucleus so yes i change it.

I suppose that collision would be automatically changing based on hex structure of organelle, if not i revert that modification.

Copy link
Member

Choose a reason for hiding this comment

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

My point is that there should be a lot of discussion going into why the hexes of the nucleus need to be changed. And until then I'm not going to accept a change here.

@@ -1034,7 +1042,7 @@ class MicrobeEditor{
private int symmetry = 0;

private bool microbeHasBeenInEditor = false;

private bool nucleusIsPresent = false;
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell this doesn't work if the nucleus is added before entering the editor.

Copy link
Contributor Author

@Dak2896 Dak2896 Dec 27, 2018

Choose a reason for hiding this comment

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

Well yes, but can nucleus be adedd before entering in editor?

Copy link
Member

Choose a reason for hiding this comment

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

You can add the nucleus, exit the editor (as you have to as you are out of MP) and then the next time everything will be locked again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why everything should be locked? you mean i can't add more organelles?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok so to summarize, if i haven't nucleus i can add only non membrane organelles, when i have nucleus i can add them.

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

@Dak2896 Dak2896 Dec 27, 2018

Choose a reason for hiding this comment

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

Ok i change how check work, is setted outside in a function that is called when you go outside of editor, so undo bug is solved.

I lock organelles, but i have no idea how to block from GUI point of view, like now you can pick an organelle but you cannot attach it, form me that i know why it's great, for player could sound like a bug

@@ -119,7 +118,6 @@ class MicrobeEditor{
// Get the species organelles to be edited
auto@ templateOrganelles = cast<array<SpeciesStoredOrganelleType@>>(
playerSpecies.organelles);

Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove completely reasonable whitespace, I can tolerate a little bit of such edits from @Untrustedlife as he has done so much.

@@ -227,7 +224,11 @@ class MicrobeEditor{

private void _addOrganelle(PlacedOrganelle@ organelle)
{
EditorAction@ action = EditorAction(organelle.organelle.mpCost,

if((organelle.organelle.name == "nucleus" && !nucleusIsPresent) || (nucleusIsPresent && (organelle.organelle.name == "mitochondrion" || organelle.organelle.name == "chloroplast" || organelle.organelle.name == "nitrogenfixingplastid" || organelle.organelle.name == "chemoplast")) || (organelle.organelle.name == "cytoplasm" || organelle.organelle.name == "flagellum" || organelle.organelle.name == "chromatophors" || organelle.organelle.name == "metabolosome" || organelle.organelle.name == "oxytoxy" || organelle.organelle.name == "vacuole" ))
Copy link
Member

Choose a reason for hiding this comment

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

This line is too long

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean as text formatting or you mean it's bad idea solve problem in that way?

Copy link
Member

Choose a reason for hiding this comment

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

Now that I read the whole line, it obviously should be a configuration property on the organelles if they need a nucleus, instead of this monstrous line here.

@@ -945,7 +962,7 @@ class MicrobeEditor{

} else if(type == "MicrobeEditorExited"){
LOG_INFO("MicrobeEditor: applying changes to player Species");

getNucleus();
Copy link
Member

Choose a reason for hiding this comment

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

I'm still not convinced that this is bullet proof. The check should happen when entering the editor. And placing a nucleus should also update it.

And to avoid confusion the buttons for organelles that can't be placed should be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the check should be made when we enter in Editor, and enabling or not some GUI buttons.
And once you have put a nucleus this functions shouldn't be call again in next edits.

Do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

The function should be called: when entering, when placing a nucleus, when redoing a nucleus action, and when undoing a nucleus action. It might be cleaner to run it when entering and then check if something changed when each action is applied or undone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, but if nucleus cost 100 points if i run it when i enter in editor and when i go out should be enough i think.

Copy link
Member

Choose a reason for hiding this comment

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

I'm considering all possible scenarios, one of which is that you are playing around with a freebuild editor, where you have unlimited MP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, in that case it won't work, the problem is that in function undo e redu scope is different, i dont't think class function can be run.

@Dak2896 Dak2896 changed the title Nuclues in Editor, Starting cell 1 hex of cytho, nucleus size 9 hex, flagella test 0.6 force Nuclues in Editor, Starting cell 1 hex of cytho Dec 28, 2018
- Possible to create nucleus
- Organelle that need nucleus are locked
@Dak2896
Copy link
Contributor Author

Dak2896 commented Dec 30, 2018

I introduce parameters to each organelles to lock/unlock nucleus, the check is made before add organelle is called, the bad things that by make the check before i should make it for all symmetry parameters.

Maybe i should make the check one time inside the function.

 - from boolean check to integer check, this handle better the checking for nucleus
@Dak2896 Dak2896 closed this Dec 31, 2018
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